Blog

FixInsight vs FMX in Delphi 10 Seattle

Several month ago I analyzed Delphi frameworks (VCL, FMX and RTL) with FixInsight and have found a number of interesting pieces of code. In this post I want to repeat my investigation and take a look at the latest FMX code in Delphi 10 Seattle Update 1 (23.0.21418.4207). As usual I will try to skip minor issues and mention only the most interesting parts of code.

W528 Variable ‘I’ not used in FOR-loop

      for I := 0 to FContent.ControlsCount - 1 do
      begin
        Ctrl := FContent.Controls[Index];
        if (Ctrl.Align = TAlignLayout.None) and ([TAnchorKind.akRight, TAnchorKind.akBottom] * Ctrl.Anchors <> []) and
          ControlList.TryGetValue(Ctrl, R) then
        begin
          // ...

This for-loop never uses ‘I’ variable. But it uses ‘Index’ instead (see line 558) which is a TFmxObject class property.

W528 Variable ‘J’ not used in FOR-loop

    // Iterate through each control's list and remove gesture ID's
    // that don't exist in the updated FCustomGestures list
    for P in FControls do
      for I := P.Value.Count - 1 downto 0 do
        for J := 0 to FRecordedGestures.Count - 1 do
        begin
          if (P.Value[I].GestureType = TGestureType.Recorded) and
            (FRecordedGestures.FindGesture(P.Value[I].GestureID) = nil)
          then
            P.Value.Delete(I);
        end;

I have no idea what’s the purpose of this nested for-loop. Let me know if it’s a bug or not.

W511 Object ‘LinePath’ created in TRY block

  for I := 0 to FFrame3D.Count - 1 do
  try
    LinePath := TPathData.Create;
    LLine := FFrame3D[I];
    LineAdvance := 0;
    LineVerticalAdvance := 0;

No, it’s not that case when you set a variable to nil before a try block :)

W508 Variable is assigned twice successively

      P := Content.LocalToAbsolute(BottomRight);
      P := AItem.ParentControl.AbsoluteToLocal(Content.LocalToAbsolute(BottomRight));

Not a bug actually, but is the first assignment really needed?

And one more:

    Difference := [];
    Difference := Value - FInteractiveGestures;

Now bad news.

Unfortunately some of the issues I have reported last time were not fixed. So I have to repeat and update.

W517 Variable ‘Color’ hides a class field, method or property

procedure TLight.ReadDiffuse(Reader: TReader);
var
  Color: Integer;
begin
  IdentToAlphaColor(Reader.ReadIdent, Color);
  {$R-}
  Color := TAlphaColor(Color);
  {$R+}
end;

Variable ‘Color’ hides the TLight.Color property. So I think line 2759 should be “Self.Color := TAlphaColor(Color);”

And the same again:

procedure TStrokeCube.ReadMaterialDiffuse(Reader: TReader);
var
  Color: Integer;
begin
  IdentToAlphaColor(Reader.ReadIdent, Color);
  {$R-}
  Color := TAlphaColor(Color);
  {$R+}
end;

W508 Variable is assigned twice successively

  FPixelShader := TShaderManager.RegisterShaderFromData('gouraud.fps', TContextShaderKind.PixelShader, '', [
    TContextShaderSource.Create(TContextShaderArch.DX9, [
      $00, $02, $FF, $FF, $FE, $FF, $32, $00, $43, $54, $41, $42, $1C, $00, $00, $00, $9F, $00, $00, $00, $00, $02, $FF, $FF, $03, $00, $00, $00, $1C, $00, $00, $00, $00, $01, $00, $20, $98, $00, $00, $00,
// skipped

  FPixelShader := TShaderManager.RegisterShaderFromData('gouraud.fps', TContextShaderKind.PixelShader, '', [
    TContextShaderSource.Create(TContextShaderArch.DX9, [
      $00, $02, $FF, $FF, $FE, $FF, $32, $00, $43, $54, $41, $42, $1C, $00, $00, $00, $9F, $00, $00, $00, $00, $02, $FF, $FF, $03, $00, $00, $00, $1C, $00, $00, $00, $00, $01, $00, $20, $98, $00, $00, $00,
// skipped

Two assignments in a row, probably not an issue, just a sloppy copy-paste.

W503 Assignment right hand side is equal to its left hand side

  if X1 > X2 then
    X1 := X1;
  if Y1 > Y2 then
    Y1 := Y2;

I guess, it should be “X1 := X2”.

      if (Self.Form <> nil) and (Self.Form.Handle <> nil) then
        Self.Form := Self.Form;

Not sure what was meant to be there (Self.Form is a record field).

And one more.

      if FNew.FFrequency <> 0 then
        FNew.FValue := Round(FNew.FValue / FNew.FFrequency) * FNew.FFrequency
      else
        FNew.FValue := FNew.FValue;

W510 Values on both sides of the operator are equal

            if RegionSize = RegionSize then
            begin
              SetLength(UpdateRects, RegionData.rdh.nCount);
              for i := 0 to RegionData.rdh.nCount - 1 do
              begin
                R := PRgnRects(@RegionData.buffer[0])[i];
                UpdateRects[i] := RectF(R.Left, R.Top, R.Right, R.Bottom);
              end;
            end;

Not sure what was meant to be there. The same code in FMX.Platform.Win.Style.pas

          if RegionSize = RegionSize then
          begin
            SetLength(UpdateRects, RegionData.rdh.nCount);
            for I := 0 to RegionData.rdh.nCount - 1 do
            begin
              R := PRgnRects(@RegionData.buffer[0])[I];
              UpdateRects[I] := RectF(R.Left, R.Top, R.Right, R.Bottom);
            end;
          end;

Copy and paste?

W513 Format parameter count mismatch

function TCustomValueRange.GetNamePath: string;
begin
  Result := Format( 'Value: %0:*.*f (%1:*.*f..%2:*.*f)', [Value, Min, Max]);
end;

The format string is incorrect. This code will raise an exception if you try to execute it.

W505 Empty THEN block

      if FoundValue.Count > 1 then
      else if FoundValue.Count > 0 then
        PropValues[Name] := FoundValue[0];

This looks strange, perhaps it could be replaced with a simple “If FoundValue.Count = 1 then PropValues[Name] := FoundValue[0]”.

That’s all. A good reason to use FixInsight, isn’t it? :)