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.
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.
Hi Robert!
ReplyDeleteI have both situation covered using SonarQube + https://github.com/fabriciocolombo/sonar-delphi plugin. This is a great tool to measure code quality and metrics.
I will have to take a look at it. I wonder how it compares to FixInsight from TMS.
DeleteI agree with your point, but Tokyo will need more than a code review, as one can see in this list:
ReplyDeletehttps://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