logo
  • Buy
  • Download
  • Documentation
  • Blog
  • Contact

Blog

27 Mar 2015

FixInsight vs VCL

by Roman Yankovsky 1 Comment

Of course I couldn’t resist analyzing Delphi VCL with FixInsight. All examples in this post are related to Delphi XE7 (version 21.0.17017.3725). I will try to choose the most interesting places and ignore minor issues like methods that are too long or nested “with” statements (have you ever thought how many nested “with” statement in VCL?).

W512 Odd ELSE-IF condition

    if Element = sbGroupStartInactive then
      LSrcRect := System.Types.Rect(0, 110, 22 - 2, 132)
    else if Element = sbGroupEndInactive then
      LSrcRect := System.Types.Rect(22, 110, 46 - 2, 132)
    else if Element = sbGroupEndInactive then
      LSrcRect := System.Types.Rect(47, 110, 70 - 2, 132)
    else
      LSrcRect := System.Types.Rect(71, 110, 95 - 2, 132);
    DrawButton(RibbonSkin.Bitmap, LSrcRect, Canvas, Rect, 2, 2, 255);

Pay attention to lines 809 and 811. These conditions are equal. There is obviously something wrong. How could we fix this?

Let’s look around:

  else if Element in [sbGroupStartInactive, sbGroupMiddleInactive,
    sbGroupEndInactive, sbGroupSingleInactive] then
  begin
    if Element = sbGroupStartInactive then
      LSrcRect := System.Types.Rect(0, 110, 22, 132)
    else if Element = sbGroupMiddleInactive then
      LSrcRect := System.Types.Rect(22, 110, 46, 132)
    else if Element = sbGroupEndInactive then
      LSrcRect := System.Types.Rect(47, 110, 70, 132)
    else
      LSrcRect := System.Types.Rect(71, 110, 95, 132);
    DrawButton(RibbonSkin.Bitmap, LSrcRect, Canvas, Rect, 2, 2, 255);
  end
  else if Element in [sbGroupStartSplitInactive, sbGroupMiddleSplitInactive,
    sbGroupEndSplitInactive, sbGroupSingleSplitInactive] then
  begin
    if Element = sbGroupStartInactive then
      LSrcRect := System.Types.Rect(0, 110, 22 - 2, 132)
    else if Element = sbGroupEndInactive then
      LSrcRect := System.Types.Rect(22, 110, 46 - 2, 132)
    else if Element = sbGroupEndInactive then
      LSrcRect := System.Types.Rect(47, 110, 70 - 2, 132)
    else
      LSrcRect := System.Types.Rect(71, 110, 95 - 2, 132);
    DrawButton(RibbonSkin.Bitmap, LSrcRect, Canvas, Rect, 2, 2, 255);
  end
  else if Element in [sbSmallSplitHoverSplit, sbSmallSplitPressedSplit,
    sbSmallSplitInactiveSplit] then

That’s even more interesting. Along with “Element in [sbGroupStartSplitInactive, sbGroupMiddleSplitInactive, sbGroupEndSplitInactive, sbGroupSingleSplitInactive]” condition (see line 804), that code looks even more strange. I guess it is a rought copy-paste of the code above (lines 794 – 802) and actually it is intended to look like this:

  else if Element in [sbGroupStartSplitInactive, sbGroupMiddleSplitInactive,
    sbGroupEndSplitInactive, sbGroupSingleSplitInactive] then
  begin
    if Element = sbGroupStartSplitInactive then
      LSrcRect := System.Types.Rect(0, 110, 22 - 2, 132)
    else if Element = sbGroupMiddleSplitInactive then
      LSrcRect := System.Types.Rect(22, 110, 46 - 2, 132)
    else if Element = sbGroupEndSplitInactive then
      LSrcRect := System.Types.Rect(47, 110, 70 - 2, 132)
    else
      LSrcRect := System.Types.Rect(71, 110, 95 - 2, 132);
    DrawButton(RibbonSkin.Bitmap, LSrcRect, Canvas, Rect, 2, 2, 255);
  end

I don’t have much experience with Ribbon controls, so the fix offered solely on the basis of common sense. Maybe someone can do it better.

OK, go ahead.

W507 THEN statement is equal to ELSE statement

It’s Ribbon again :)

procedure TRibbonDropDownButton.DrawBackground(var PaintRect: TRect);
var
  LPt: TPoint;
begin
  inherited;
  LPt := GetArrowPosition(PaintRect);
  if not Enabled then
    DrawArrow(LPt, clNone, clNone, -1)
  else
    DrawArrow(LPt, clNone, clNone, -1);
end;

No comments. Obviously, the code in “then” and “else” are the same. It’s not clear what was meant to be there.

A similar issue in another file:

      if Assigned(Clients[I].ActionBar) then
      begin
        if Assigned(ActionProc) then
          ActionProc(Clients[I])
        else
          Clients[I].Refresh;
        UpdateActionBar(Clients[I]);
      end
      else
      begin
        if Assigned(ActionProc) then
          ActionProc(Clients[I])
        else
          Clients[I].Refresh;
        UpdateActionBar(Clients[I]);
      end;

And one more:

        if Style <> csSimple then
          if DroppedDown then
            DrawFrameControl(C.Handle, R, DFC_SCROLL, DFCS_FLAT or DFCS_SCROLLCOMBOBOX)
          else
            DrawFrameControl(C.Handle, R, DFC_SCROLL, DFCS_FLAT or DFCS_SCROLLCOMBOBOX);

OK, next.

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

function CheckPoint(const Point, Source: TPoint; Deviation,
  ErrorMargin: Integer): Double;
var
  Distance: Double;
  ErrorMarginDistance: Double;
  M, B: Double;
begin
  Deviation := Deviation;
  Distance := Sqrt(Sqr(Point.X - Source.X) + Sqr(Point.Y - Source.Y));
  ErrorMarginDistance := MulDiv(Deviation, ErrorMargin, 100);

This assignment either should be different or it is completely useless.

And one more:

procedure TScreenTipsWindow.CreateParams(var Params: TCreateParams);
begin
  inherited CreateParams(Params);
  with Params do
  begin
    Style := WS_POPUP;
    WindowClass.Style := WindowClass.Style;
    if NewStyleControls then
      ExStyle := WS_EX_TOOLWINDOW;
    if CheckWin32Version(5, 1) then
      WindowClass.Style := WindowClass.Style or CS_DROPSHADOW;
    if NewStyleControls then
      ExStyle := WS_EX_TOOLWINDOW;
    AddBiDiModeExStyle(ExStyle);
  end;
end;

And last, but not least…

W504 Missing INHERITED call in destructor

There are too many messages of this type, so I’m not going to quote everything.

destructor TActiveXPropertyPage.Destroy;
begin
  FPropertyPageImpl.FPropertyPage.Free;
  FPropertyPageImpl.Free;
end;

There is no “inherited” call, ancestor’s destructor is never called.

Well, let’s finish. As I said at the beginning, I don’t want to nitpick, so I ignored minor issues. In the next episode, we may talk about FMX or RTL :)

Static analysis is cool, isn’t it?


  • Wouter says:
    March 28, 2015 at 9:44 pm

    Wow, nice! I wonder what I’ll find in my own code. The issues that you’ve found are easy to miss if you just scan code with your eyes only.

    Reply

Leave a Reply Cancel reply

Your email address will not be published. Required fields are marked *

  • Announcements
  • DelphiAST
  • DelphiSpec
  • FixInsight
  • FMX
  • Other
  • VCL

Recent Posts

  • Find leaks in Delphi with Deleaker
  • FixInsight and the inline directive
  • FixInsight 2017.04 support Delphi 10.2 Tokyo
  • FixInsight 2016.04 support Delphi 10.1 Berlin
  • FixInsight vs FMX in Delphi 10.1 Berlin

Archives

  • January 2020
  • April 2017
  • April 2016
  • March 2016
  • December 2015
  • November 2015
  • October 2015
  • September 2015
  • August 2015
  • April 2015
  • March 2015
  • February 2015
  • September 2014
  • August 2014
  • January 2014
  • December 2013
  • October 2013

Recent Comments

  • anapa-poseidon3.ru on FixInsight vs RTL
  • Heat pump on FixInsight vs RTL
  • Suing on FixInsight vs RTL
  • JorgeJag on Find leaks in Delphi with Deleaker
  • Prorabdom on FixInsight vs RTL
  • Home
  • Buy
  • Download
  • Documentation
  • Blog
  • Contact
  • © 2014-2015 SourceOddity|
  • Terms and Conditions|
  • Privacy Policy