Tuesday, March 28, 2017

The Value of Code Reviews

I have recently seen code that that looked like this.
 
try
  ... Lines of Code here ...
except
end;
or
 
procedure TCustomClass.MethodDoThis()
begin
  ... 20 Lines of code ...
end; 

procedure TCustomClass.MethodDoThis2()
begin
  ... Same 20 Lines of code with slight modification ...
end;
Both produced code that may have passed end user testing. But both cause long-term problems.
  • The first example was just hiding all exceptions, a bad practice. After the code review, it was determined that the try | except block was not even needed.
  • The second example was refactored to reduce the need for duplicate code. The duplicate code causes problems when someone has to make changes that would apply to both methods but does not notice the 2nd method and therefore neglects to change it.
My point is simple, code reviews have value.

Sidenote: Looking forward to reviewing Tokyo release of Delphi soon. With 500+ Bug Fixes with many in areas that impact our system is definitely worth investigating. Security and Quality go hand in hand, and I appreciate work to improve security. Typically, security issues are related to bugs in code that are exploited by hackers.

3 comments:

  1. Hi Robert!

    I have both situation covered using SonarQube + https://github.com/fabriciocolombo/sonar-delphi plugin. This is a great tool to measure code quality and metrics.

    ReplyDelete
    Replies
    1. I will have to take a look at it. I wonder how it compares to FixInsight from TMS.

      Delete
  2. I agree with your point, but Tokyo will need more than a code review, as one can see in this list:
    https://quality.embarcadero.com/browse/RSP-17676?jql=(type%20%3D%20%22bug%22)%20and%20(%20(affectedVersion%3D%2210.2%20Tokyo%22)%20or%20(%22Build%20No%22~%2225.0.26309.314%22)%20)%20ORDER%20BY%20issueKey%20DESC

    ReplyDelete