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);
var
  P: array[1..3] of TPoint;
  AColor: COLORREF;
begin
  with MarkR, P[1] do
  begin
    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);
  end;
  // more code here.
end;

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
begin
   point(gun,foot);
   fire();
end.

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?


29 comments:

  1. I hate as much as you, by the name of the method I found: @Dxbar@TdxBarPainter@BarDrawMarkArrow

    ReplyDelete
  2. I avoid using pointers, especially untyped ones.
    This also makes sure that I don't get c-like memory management all over my code.

    And instead of having global constants, variables, and functions, I group them in classes as class const, class var and class function.

    If you declare a global parameterless function or a global var foot or gun, you can have the same scoping confusionn as described above, without any with statements. By grouping stuff in a class, you're forced to be explicit.

    ReplyDelete
    Replies
    1. I was with you with the first part about pointers.

      As to the second, objects are supposed to be data and the methods that act on them. I've always believed that classes without data or without methods are heresy created by Java and then C# to be "pure" object oriented at the expense of destroying the notion of an object.

      I don't understand how a global function can cause the namespace mangling that the "with" statement does. Now, Delphi's unit imports that allow namespace collisions are another story and perhaps that's what you're trying to avoid.

      Delete
  3. I hate to have multi private, protected, public... in one class.

    ReplyDelete
  4. Works good in Freepascal but I must confess if you don't now the members or methods offered you can run into troubles with the Forms members - which is correct in general but somehow confusing when you rely on Code Completion.

    ReplyDelete
  5. This comment has been removed by the author.

    ReplyDelete
  6. YAWN - yet another With nag ;)

    But I have to agree, for all the noise about the With statement in general, this is by far the worst usage I've seen of it. Many years ago as part of a standards document I specifically highlighted that particular usage and banned it!!

    ReplyDelete
  7. Never use Generics. Templates are one of the worst things C++ has (it gave me more headaches than solutions) and I really don't know what it provides that classes and/or interfaces have yet.

    Today I discover that you can create generic interfaces. Who thought that's a good idea?

    ReplyDelete
    Replies
    1. Did you timetravel from the last century to write this comment?
      Generics != Templates in C++. Yes, they work very similar but that's it.

      Generics make code so much more readable and prevent you from writing the similar code over and over (type safe lists without generics anyone?). I am not talking about other things you can do with generics that would require tons of code every time without them.

      The same goes for generic interfaces. If you ever worked with interfaced generic collections you would know how nice it is to have type safety and the automatic memory management that interfaces provide.

      Delete
    2. "Did you timetravel from the last century to write this comment?" For a second I though you'll say that C++ Templates were changed to much with the C99 standard that are actually useful (as well as the "zero array").

      Anyway, I did read a lot about generics in Free Pascal's developers mailing lists (the guys that are working on the compiler, not the ones that are working with the compiler) and I didn't see any difference. Ok, Delphi isn't FPC but...

      May be I should look for a paper explaining the differences between Object Pascal's Generics and C++'s Templates...

      Delete
    3. My comment was more about thinking that generics are not a good idea - especially generic interfaces - I even wish I could have generic methods on interfaces in Delphi!

      There is an excellent article explaining the differences:
      http://blogs.teamb.com/craigstuntz/2009/10/01/38465/

      I have my opinion on FPC and the features they implement (or I should say they do *not* implement). While it has some nice things that Delphi does not there are things missing that make FPC look like it's from the stone age. And generics (or the lack of) is one of these things. I know they are being worked on in 2.7.1 but they are far from being any useful except you just use them for some lists which seems to be the example everyone gives when it comes to point out what generics are useful for.

      Yes, generics have some issues (as I wrote in my blog some weeks ago) but the benefits by far outweigh them (I guess I should write an article about that ^^).

      Delete
    4. @Stefan "Generics make code so much more readable and prevent you from writing the similar code over and over "

      Stefan, where are you on Stack Overflow? Certain people have told me there that there's absolutely no violation of Don't Repeat Yourself in Pascal. I've yet to read their replies to my rebuttal because I'm sure it will drive my blood pressure up. :-(

      "While it has some nice things that Delphi does not there are things missing that make FPC look like it's from the stone age. "

      Once upon a time FPC found itself light-years ahead of Delphi in 64-bit, cross-platform, etc. Now it seems development has slowed down significantly and they've even gotten behind in areas such as generics and Unicode. I was shocked to discover that there's essentially no generic containers in the Lazarus standard library.

      Delete
  8. I hate poor developers who wants to be sandboxed by the language they use because they are too lazy to read the documentation and can easily fire in their feet while coding lazily.
    So they want advanced, powerful feature removed, and all languages should become a copy of Visual Basic so they can't cause harm to themselves.
    Don't you like with? Don't use it. Don't you like pointers? Don't use them. Just, let those who are able to take advantage of them - and learnt how to use them properly - use them as they see fit. Don't you like powerful languages? There are a lot of simpler less powerful ones to use, select one, and be happy.

    ReplyDelete
    Replies
    1. Pointers are useful. `with` is more harm than good.

      Delete
    2. With can be useful as wel. When you have a deep hierarchy of objects to access, say object1.object2.object3. writing
      with object1.object2.object3 do
      begin
      Prop1 :=
      DoSomenthing(a, b, C);
      etc. etc.
      end;
      *is* useful and means less code clutter than repeating
      object1.object2.object3....
      object1.object2.object3....
      object1.object2.object3...
      especially when identifier are long names like MyVeryBeautifulObject.ILikeInfixCaps.BecauseItDeliversGreatIdentifiers
      Then you can use with to create havoc in your code, true.

      Delete
    3. Law of Demeter aside, for deep hierarchies, I prefer using an explaining variable instead of WITH. Accomplishes the same thing, is more readable and avoids accidental ambiguity.

      Delete
    4. While littering your stack (and method declarations) - especially if you have different types to handle, and maybe bypassing some compiler optimizations ;)
      Again, it's something to use sparingly, but why not?

      Delete
    5. @kmorwath: when I say that I avoid with statements, untyped pointers or c-style memory management, or c-style string handling, it's not because I don't read documentation or because I'm lazy, but because there are usually better ways to get the same thing done in Delphi.

      I'm not advocating to remove these features from the language; I'm just trying to point out that they are code smells to me, but we all have our own styles and standards.

      Delete
    6. "better ways to get things done" depends what "better" means for you in a given situation. Better readability? Better maintenability? Ease of writing? Or better perfomances? Or better applications? There are situation where untyped pointers and manual memory management are the only ways - and the better ones - to achieve some functionalities without writing ugly, non performant code (as the one that is now crippling the RTL...). Face it - CPUs still don't understanding anything more than pointers to block of memory. Sure, then there are those who use(d) string as memory buffers - just because they were afraid of pointers and GetMem()...

      Delete
    7. @hmorwath: In my book, readable and maintainable code trumps "clever" code every time.

      Delete
    8. >I hate poor developers who wants to be sandboxed by the language
      >they use because they are too lazy to read the documentation

      I actively dislike the RTFM excuse for poor design. In design (UI, API, etc.) it's a truism that the right/safe way to do something should always be the easy way to do it. Anything else is encouraging improper behavior. Sorry, RTFM doesn't excuse poor design.

      >So they want advanced, powerful feature removed,

      1) With is 200 years old... or so it seems. If it was in Turbo Pascal, it's certainly not "advanced". And powerful? It tempts you to type less at the expense of the potential for subtle, hidden errors. That's not powerful, that's dangerous.

      2) If it was so advanced and powerful Microsoft, Oracle, Larry Wall, Guido Van Rossum, etc. would have copied it for their own languages. Users of other products would be demanding "with" in their languages. I don't see that happening. Anders didn't even bring it to C#. Meanwhile it was in Visual Basic. Draw what conclusions from that you wish. :-)

      > and all languages should become a copy of Visual Basic so they can't
      >cause harm to themselves.

      You have a problem with eliminating the potential for various types of errors where possible? Would you like to go into industry and pitch them ideas for languages with more potential boobytraps? I've given tech presentations to businesses. Let me know how that goes.

      You do know you're using a statically typed language, right? Do you sneer at that because it's designed to eliminate a potential class of error?

      >Don't you like with? Don't use it. Don't you like pointers? Don't use
      >them.

      No man is an island. We have to use libraries written by people who used "with". We have to read code from people who use pointers. Heck, there's one guy on the EMBT forum who said he avoids using "string" and uses "PChar" whenever possible! Sorry, but I don't want to have to be anywhere near that kind of code.

      >Just, let those who are able to take advantage of them - and learnt how
      >to use them properly - use them as they see fit.

      They're rife for error, and you're not The Woz. Why does this always turn into an egotistical battle where someone proclaims that they love hard/dangerous/poorly designed item X because they're so smart and the great unwashed masses just can't cope?

      >Don't you like powerful languages?

      Let's not get into a discussion about where Pascal stands on the pecking order of "powerful languages" or the LISP fans will be left rolling in the aisles with laughter. I ensure you LISP users will show you more powerful constructs than "with", and fans of functional languages such as Haskell and OCaml will demo some amazingly powerful recursive techniques. Characterizing Delphi as the pinnacle of power that dummies are trying to water down is not going to gain any traction whatsoever. "With" doesn't offer any functionality whatsoever; it's merely a shorthand convenience. If you equate that with "power", there's a problem, Maybe you need to spend some time with other languages to get some perspective.

      > There are a lot of simpler less
      >powerful ones to use, select one, and be happy.

      Almost everyone already selected simpler more powerful ones and that's why Delphi is a niche language today. Let's not cling to our mistakes or refuse to admit there were any. "With" was a bad idea. Let's fix it.

      Delete
  9. I just spent some time wading through a bunch of code that uses nested WITH statements, and I'm with you brother.

    Using WITH in code that will be shared with other people is irresponsible.

    ReplyDelete
  10. "WITH a,b" should never never be used. There is no excuse for that. I personally use and never had problems with simple "WITH a DO" in small blocks of code, though. I think it is even easier to read than myclassA.Something.SomeProperty repeated over and over and over again.

    ReplyDelete
  11. I've written horrible with x,y do code in the past (many years ago), I came across one the other day and definitely had a physical reaction to it - It is amazing how badly code can be written when you are a beginner.

    ReplyDelete
  12. DWS recently added WITH support, but Eric actually got it right. The syntax looks like this:

    with shortcut := Big.long.expression.you.dont.want(to, retype) do
    shortcut.doSomething;

    You can even put multiple assignments in the with statement, but since it's based on explicit shortcut variable names and not implicit scoping, you don't run into the ugly messes you get in Delphi code.

    ReplyDelete
    Replies
    1. That is just because DWS supports inline variables. To me that has no right to exist because you can type the same by using a normal assignment and then using that variable.

      Delete
  13. I'm so smart using "WITH a,b", so smart...

    ReplyDelete