Wednesday, June 4, 2014

I officially hate hate hate the WITH statement.

This hilariously awful piece of code is from FooBars (fobar.pas), the name of the vendor has been removed so I can feel better about placing it up here to be openly criticized:

procedure TfooBarPainter.BarDrawMarkArrow(ABarControl: TfooBarControl;
  DC: HDC; MarkR: TRect);
  P: array[1..3] of TPoint;
  with MarkR, P[1] do
    P[1] := Point((Left + Right - MarkSizeArrowX) div 2,
      (Top + Bottom - MarkSizeArrowY) div 2 + (Top + Bottom - MarkSizeArrowY) mod 2);
    P[2] := Point(X + MarkSizeArrowX - 1, Y);
    P[3] := Point(X + MarkSizeArrowX div 2, Y + MarkSizeArrowY - 1);
  // more code here.

So sometimes I have used WITH in my code.  But when I read code like the one above, I want to throw up a bit.

I'm a bit shocked at how ugly and unreadable a "WITH a,b" makes the code above, and how my brain just about short-circuits when I spot one of the expressions on the outside of the with clause, inside it as well.

I'm a bit puzzled how anyone thought that it was worth writing code like this.

With foot, gun do

When you make a feature possible in a language it can never ever be removed, it would seem.  But maybe it would be nice if use of With could become a compiler warning (optionally turned on) so that those who don't want it in their code can have some compiler help in finding them and rooting them out.  In the case above,  the code is from a very well known component developer.   I think the above extremely ugly, and suspect that judicious use of temporary variables referring to objects is a very inexpensive and readable way of writing code.

But I wonder, since WITH is ancient history for most of us, we've all long since stopped using it, if there are other unreadable areas in our code.  I think there are:

  •  Every USES statement with more than some X number of  items becomes an unreadable mess.   Have you ever tried organizing your uses clauses into sections? I find that the advent of unit namespace prefixes on unit names (WinApi.Windows) combined with grouping units by their unit namespace prefixes, really increased the sense of order in my applications.   A disorganized unordered list of 30 units is unreadable and inscrutable, but a uses clause organized into RTL, VCL, ThirdParty, the MyApp units, becomes instantly clearer.
  •  Every class with 100 or more methods in it becomes completely opaque and very hard to reason about.   Restricting method visibility (no unnecessarily public methods, no unnecessarily protected methods), helps a lot.
  • I find reintroduce, and overloaded methods add a lot to the mental noise and confusion in my head, and when I can, I like to get rid of them.
Any other things you wish to banish from your own personal Delphi coding standards?