FixInsight vs VCL

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?