Some Issues Reported by FixInsight Static Delphi Code Analyser


Fixinsight is a good Delphi static code analyser. It is easy to use and well integrated into the Delphi IDE. It supports up to the latest releases of Delphi, e.g. Delphi 10.1 Berlin. The tool allows you to do one click in Delphi IDE or you can use it in command line.

The following is based on FixInsight April/2016 version.

CONST missing for unmodified string

[FixInsight Optimization] Logo.pas(796): O801 CONST missing for unmodified string parameter ‘idx’.

function in_array(idx: string; const arr: array of string): boolean; overload;
var
  i: integer;
begin
  for i := 0 to High(arr) do
  begin

This is something I need. Without const/var keyword, the string will be passed by value (another copy), so passing a reference is faster.

Unneeded boolean comparision

[FixInsight Convention] Logo.pas(2300): C109 Unneeded boolean comparison.

if (Self._running = false) then

FixInsight suggests to alter this, maybe make it like this:

if (not Self._running) then

Then is equal to ELSE

I don’t get this warning on FastMM4.991 line: 10499

[FixInsight Warning] FastMM4.pas(10499): W507 THEN statement is equal to ELSE statement

fixinsight-then-else Some Issues Reported by FixInsight Static Delphi Code Analyser code review delphi FixInsight static code analyser

fixinsight-then-else

Variable not used in FOR-loop

Considering the following:

while q.Count > 0 do
begin
  np := q.Pop;
  Dispose(np);
end;
q.Free;

Since the q.Count is invoked every time in the while loop, we can optimise this if no other threads are altering this at the same time. In fact, the queue object q is defined locally in a function, so we can safely optimise to the following FOR-loop. In Delphi, the for loop is more efficient than the while loop, because the q.Count is only evaluated once in for-loop.

for i := 1 to q.Count do
begin
  np := q.Pop;
  Dispose(np);
end;
q.Free;

However, FixInsight is clearly not very happy about this: [FixInsight Warning] RPM_Antennas.pas(1124): W528 Variable ‘i’ not used in FOR-loop. In fact, I think this rule is very difficult to check because most of the cases, it is reasonable not using the variable in the for-loop. For example, the following was from SynLZ.pas (the famous compression algorithm).

procedure movechars(s,d: PAnsiChar; t: integer); {$ifdef HASINLINE}inline;{$endif}
// fast code for unaligned and overlapping (see {$define WT}) small blocks
// this code is sometimes used rather than system.Move() by decompress2()
var i: integer;
begin
  for i := 1 to t do begin
    d^ := s^;
    inc(d);
    inc(s);
  end;
end;

Clearly, the variable t defines how many steps, which isn’t necessarily used in the loop.

–EOF (The Ultimate Computing & Technology Blog) —

GD Star Rating
loading...
604 words
Last Post: How to Remove Product Review in Woocommerce/Wordpress?
Next Post: Review: Lenovo ThinkCenter M900 Tower Station

The Permanent URL is: Some Issues Reported by FixInsight Static Delphi Code Analyser

Leave a Reply