Most of managers think that code review (Pair
Programming-PP, is one type of this) is an activity which does not generate a valuable
product and wastes valuable time of senior
employees. From their point of view, the resources are important and should be
handled carefully. They try to utilize single moment of the developers’ time
and think that how can be filled by a piece of work. Making some reviews regarding
to source code that is developed (or in general, any activity in software
development process, like testing or analyzing) does not affect the
functionality of program. If there is a bug in the code, it will be caught by
SQA (software quality assurance teams, simply testers). So there is no need such a worthless, time
consuming process.
There are also some senior employees who
are not willing to share their experiences to the junior ones. In general, they think that if the other developers
have become their knowledge, they can lose their job. Always job loss is not a
factor, losing position inside the organization creates their unhappiness.
Another aspect of this reluctance is how they achieve this knowledge. Most of
often it is because of when they are junior; the senior ones would not help
them. Somehow, they take vengeance on their pastJ. One
another view that is proposed by senior ones is seniority is a level which is
gained by hard-working, deep research via Google and reading some cool software
books. The only way to get an expertise level on a subject is doing all of
these rituals. After time (years and years) passes, if they are patience,
junior ones will grow up and become senior employee. The patience will
increment their degree of expertise.
Actually, now, I will share my experiences
with this important software process. What I learned until now is that pair programming
(or in general code reviews) will grow people (especially developers) very
quickly and helps projects to build with less bugs/SLOC (source lines of code)
software metric. Although it consumes some more time for reviewing processes,
overall gain in [bug-fix-test] cycle will compensate it, easily.
Code review (or in high level form, review
or inspection) is a software engineering concept that is mainly focuses on
quality. Software quality is the one which should be on top-ten list of good
software product features. Whereas review is usually done for source code, some
other software artefacts -like design- can be reviewed, as well. Because of
being human, we can always make mistakes. With review process, it is aimed that
mistakes which are possible to happen during software development life cycle
(SDLC), can be investigated early, before building some other codes on the top
of it. The sooner a bug or an improvement point is realized, the easier it is
solved or enhanced. From this perspective, code review can be assumed as a
pre-unit-testing stage, most helpful unit testing without writing any test
codesJ.
One other important benefit is improving
developer while the software project is implemented. Not only less-experienced
developer improves his abilities regarding source code quality but also expert
developer or architect improves the capabilities of presentation, persuasion
and self-expression. If expert one requires a change on the source, he should
have good reasons in order to convince the junior. If he cannot tell the reason
of change, it will not be possible to impose his ideas. After this point,
senior developers start to make some research and renew his knowledge about
language of programming and general software design concepts for changing minds
of junior programmers.
There are many formal (audit, independent
organization for review; inspection, developer presents his code to a dedicated
review group in a meeting and usually a written report is generated) and
informal (deskckeck, developer sends the code to reviewers then feedbacks are
sent back; walkthrough, author of code presents his code to the –mostly- inexperienced
people; code review, an experienced developer or team checks your code with you
and they give some valuable advises to improve code quality; peer-to-peer or
pair programming, two developers work on the same code) ways. Pair programming
is the most informal and warm type of review. I like it too much, I confessJ. 2 developers sits on a single computer and one of them is
developing source code and the other one is checking and giving some advises
about code. After a while, the hats are changing and the one who was developing
becomes controller, advisor. By the help of this type of programming, at least
two different developers will be able to maintain the code, later. They will be
their backups, which is an important point. For instance, if one of them is
ill, the other can still continue development process. Or if one of them left
the job, there is no need to knowledge transfer to another one. If pairs are
changing for 3 months in circular fashion, all developers will get knowledge
about the software products those are developed in the house. In addition, if developers have some mental
weakness, it will be difficult to create pairs. Some coders do not like working
together; they are more productive when they develop the code alone. There can
be some other pros and cons of pair programming.
What makes this article funny is Extended
Pair Programing that is mentioned in the subject. What we are doing in the real
life is that we are using Pair Programing techniques in a different way. I am
calling it extended. When we are responsible to write any lines of codes, we
are assigning it to less-experienced developer (say person J). And the job has
another responsible that has expertise level on PL/SQL language fundamentals, software
engineering concepts and the business knowledge (say person S). Firstly, J
comes to desk of S and they make a small meeting on the desk. They are talking
about the work to be done and some informal notes are taken on the paper. Then,
J goes to his computer and starts to develop the code. After J finishes the PL/SQL
package of the work, he compiles it in the database and comes to desk of S. S
makes code review of the package and writes some comments on the code and tells
to J why the code should be changed. For instance, some codes need to be refactored
or code has some performance bottlenecks or business defects. This part is
important. S does not change the code, only writes comments and compiles it into
the database again. J goes to his desk again, changes the code that should be
changed in the light of S’ advises. J changes the code himself for better
understanding mistakes which he has made. Then compiles it in the database and comes
to S again… this cycle continues until the code is fully utilized in terms of
source code quality. You can think that this process wastes developer’s time;
but I can express that this process does not spend time. I have applied this
process with other colleagues. I saw the improvement of colleagues and the quality
with shipped software product. If we did not develop PL/SQL packages with this
technique, usually bugs are detected in the testing process or in the
production environment. When a new request comes to that package, it becomes
difficult to respond the change due to some bad-designed codes. During time,
this review process takes only one shot.
Sometimes you do not need to change anything, because developers learn
your style of review and make necessary changes on the code before review process.
If a colleague enters this type of
development initially, he just sits and looks the code that is developing by
senior one. After first or second work, he starts to develop himself.
I will demonstrate a real example in order
to show you how this process can be handled:
Business Requirement: Generate a PL/SQL
package which returns free minutes of a mobile subscriber.
Phase 1: Less-experienced developer (say
person J) comes to desk of expert (say person S) person to talk about new
requirement. J understands what he should develop and takes some notes
regarding to design and code. Then J goes to his computer and starts to write
codes.
Phase 2: J finishes the PL/SQL package and
compiles it into database, and then comes to S to check the code. The code
looks-like:
CREATE OR REPLACE PACKAGE BODY
FREE_UNIT_QUERY IS
FUNCTION
get(pin_SubscriberId IN NUMBER) RETURN NUMBER IS
vn_Free NUMBER;
BEGIN
SELECT (nvl(b.granted, 0) - nvl(b.consumed, 0)) /
60
INTO vn_Free
FROM balance b
WHERE b.subscriber_id =
pin_SubscriberId
AND
b.billing_period =
(SELECT MAX(b2.billing_period)
FROM
balance b2
WHERE
b2.subscriber_id = b.subscriber_id);
RETURN vn_Free;
END get;
END FREE_UNIT_QUERY;
Phase 3: S and J review the package
together. After review process, code is commented like that:
CREATE OR REPLACE PACKAGE BODY
FREE_UNIT_QUERY IS
FUNCTION get -- MT: change to a meaningful name
(pin_SubscriberId IN NUMBER -- MT
: use indirect type definitions
) RETURN NUMBER IS
vn_Free NUMBER; -- MT : change variable name
BEGIN
--- MT : refactor it to function
SELECT (nvl(b.granted, 0) - nvl(b.consumed, 0)) /
60
INTO vn_Free
FROM balance b
WHERE b.subscriber_id =
pin_SubscriberId
AND
b.billing_period =
(SELECT MAX(b2.billing_period)
FROM
balance b2
WHERE
b2.subscriber_id = b.subscriber_id);
RETURN vn_Free;
END get;
END FREE_UNIT_QUERY;
Phase 4: J checks the comments and re-write
the code. Finally he goes to S with following code:
CREATE OR REPLACE PACKAGE BODY
FREE_UNIT_QUERY IS
FUNCTION GetRemainingFreeUnitsInSeconds(pin_SubscriberId
IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInSeconds NUMBER;
BEGIN
SELECT (nvl(b.GRANTED, 0) - nvl(b.CONSUMED, 0))
INTO vn_FreeUnitsInSeconds
FROM BALANCE b
WHERE b.SUBSCRIBER_ID =
pin_SubscriberId
AND
b.BILLING_PERIOD =
(SELECT MAX(b2.BILLING_PERIOD)
FROM
BALANCE b2
WHERE
b2.SUBSCRIBER_ID = b.SUBSCRIBER_ID);
RETURN vn_FreeUnitsInSeconds;
END GetRemainingFreeUnitsInSeconds;
FUNCTION
GetFreeMinutes(pin_SubscriberId IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInMinute NUMBER;
BEGIN
vn_FreeUnitsInMinute := GetRemainingFreeUnitsInSeconds(pin_SubscriberId)
/ 60;
RETURN vn_FreeUnitsInMinute;
END GetFreeMinutes;
END FREE_UNIT_QUERY;
Phase 5: S and J review the package again. After
review process, code is commented like that:
CREATE OR REPLACE PACKAGE BODY
FREE_UNIT_QUERY IS
FUNCTION GetRemainingFreeUnitsInSeconds(pin_SubscriberId
IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInSeconds NUMBER;
BEGIN
--
MT : use rank for performance and check if it is better
SELECT (nvl(b.GRANTED, 0) - nvl(b.CONSUMED, 0))
INTO vn_FreeUnitsInSeconds
FROM BALANCE b
WHERE b.SUBSCRIBER_ID =
pin_SubscriberId
AND
b.BILLING_PERIOD =
(SELECT MAX(b2.BILLING_PERIOD)
FROM
BALANCE b2
WHERE
b2.SUBSCRIBER_ID = b.SUBSCRIBER_ID);
RETURN vn_FreeUnitsInSeconds;
END GetRemainingFreeUnitsInSeconds;
FUNCTION
GetFreeMinutes(pin_SubscriberId IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInMinute NUMBER;
BEGIN
vn_FreeUnitsInMinute := GetRemainingFreeUnitsInSeconds(pin_SubscriberId)
/ 60; --
MT : use TRUNC
RETURN vn_FreeUnitsInMinute;
END GetFreeMinutes;
END FREE_UNIT_QUERY;
Phase 6: J checks the comments and re-write
the code. Finally he goes to S with following code:
CREATE OR REPLACE PACKAGE BODY
FREE_UNIT_QUERY IS
FUNCTION GetRemainingFreeUnitsInSeconds(pin_SubscriberId
IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInSeconds NUMBER;
BEGIN
SELECT (nvl(inn.GRANTED, 0) - nvl(inn.CONSUMED, 0))
INTO vn_FreeUnitsInSeconds
FROM (SELECT
b.*,
rank() over(PARTITION BY b.SUBSCRIBER_ID ORDER BY b.BILLING_PERIOD DESC)
rnk
FROM
BALANCE b
WHERE
b.SUBSCRIBER_ID = pin_SubscriberId) inn
WHERE rnk = 1;
RETURN vn_FreeUnitsInSeconds;
END GetRemainingFreeUnitsInSeconds;
FUNCTION
GetFreeMinutes(pin_SubscriberId IN BALANCE.SUBSCRIBER_ID%TYPE)
RETURN NUMBER IS
vn_FreeUnitsInMinute NUMBER;
BEGIN
vn_FreeUnitsInMinute := TRUNC(GetRemainingFreeUnitsInSeconds(pin_SubscriberId)
/ 60);
RETURN vn_FreeUnitsInMinute;
END GetFreeMinutes;
END FREE_UNIT_QUERY;
Phase 7: Everything is OK. Code is ready to
test.
No comments:
Post a Comment