Wednesday, September 21, 2016

Delphi Features I Avoid Using And Believe Need to be Gradually Eliminated from Codebases

My guest post from L.V. didn't seem to have enough Delphi specifics for one commenter, so I thought about it, and realized, that what L.V. is talking about is Practices (stuff people do), not features.

But there are features in Delphi that I think are either over-used, or used inappropriately, or used indiscriminately, or which should almost never be used, since better alternatives almost always will exist.  Time for that list. No humorous guest-posting persona for this post, sorry, just my straight opinions.

1. WITH statement

This one is hardly surprising to be in the list, as it's one of the most controversial features in the Delphi language. I believe that it is almost always better to use a local variable with a short name, creating an unambiguous and readable statement, instead of using a WITH.  A double with is much more confusing than a single WITH, but all WITH statements in application layer code should be eliminated, over time, as you perform other bug fix and feature work on a codebase.

2. DFM Inheritance

I don't mind having TApplicationForm  inherit non-visually from a TApplicationBaseForm that doesn't have a DFM associated with it, but I find that maintenance and ongoing development of forms making use of DFM inheritance is problematic.  There can be crazy issues, and it's very difficult to make changes to an upstream form and understand all the potential problems downstream. This is especially true when a set of form inheritances grows larger.     I have even forced non-visual-inheritance using an interposing class, and found that IDE stability, and ease of working with a codebase is improved.

3. Frames

The problems with frames and with DFM-inherited are overlapping, but Frames have the additional troubling property of being hard to make visually fit and look good.  You can't really assume that any change in the base frame control's original positions will be overridden or not, you just don't know. Trying to move anything around in a frame is an exercise in frustration.  I prefer to compose parts of complex UIs at runtime instead of at designtime.

4. Visual Binding

I have had nothing but trouble with Visual Binding.  It seems that putting complex webs of things into a design-time environment is not a net win for readability, clarity, and maintainability. I would rather read completely readable code, and not deal with bindings.  Probably there are some small uses for visual binding, but I have not found them. My philosophy is to avoid it. It's a cool feature, when it works.  But the end result is as much fun as a mega-form.

5. Untyped Parameters in User Procedures or Functions

Modern Pascal should be done with PByte rather than the old way of handling "void *" types (if you know C) in Pascal is the untyped var syntax. If possible, I prefer to use PByte which I consider much more modern way of working.  I believe the two are more or less equivalent in capabilities, and that Delphi still contains untyped var params for historical compatibility reasons, but unless I'm writing a TStream and must overload a method that already has this signature, I prefer not to introduce any more anachronisms like that in my code.

6. Classic Pascal File IO Procedures

Streams should have replaced the use of AssignFile, Reset, Rewrite, CloseFile.

7. Unnecessary Use of Pointer Types and Operations in Application Layer Code

In low level component code, with unit tests, perhaps sometimes, pointer types and operations will be used. To implement your own linked list of value types which are not already implicitly by reference, but in application layer (form, data module) code that most Delphi shops spend 90% of their time, introducing raw pointer operations into the application code is almost always going to make me require it to be changed, if I'm doing a code-review.   Delphi is a compiled "somewhat strongly typed" language, and I'm most happy with application layer code that does not peel away the safety that the type system gives me.

8. Use of ShortString Types with Length Delimiters, in or out of Records

Perhaps in the 1980s, a pascal file of a record type, with packed records made sense. These days, it's a defect in your code.  The problem is once such a pattern is in your code, it's very difficult to remove it.  So while an existing legacy application may contain a lot of code like that, I believe a "no more" rule has to be set up, and module by module, the unsafe and unportable stuff will have to be retired, replaced, or updated.  The amount of pain this kind of thing causes in real codebases that I have seen that used it, is hard to overstate.

9. Use of Assignable (Non Constant) Consts

A compiler flag {$J+} in Delphi allows constants to be overwritten. It should never be used.




29 comments:

  1. We have a project with a large number of forms with DFM inheritance. I appreciate the item #2 that it becomes unwieldy. Can you expand on your statement of using non-visual-inheritance using an interposing class?

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

    ReplyDelete
  3. I'll write that up in a future blog post. But basically you open the .dfm and change the "inherited" opening keyword to "object", then you go in your unit (.pas) file and write something like this:

    type
    TXForm = TMyRealFormName // Interposing class. I prefer the convention TXForm (X for extended) to overloading TForm in this context.

    TMyForm = class(TXForm)
    ...
    end;


    Next time I view the form I must remove all inherited-links from the DFM which the IDE will helpfully prompt me about. It's the ONE time when the "delete crap from DFM that I don't know what to do with" behaviour of the IDE is actually useful.

    ReplyDelete
    Replies
    1. so you put all your super class code that would normally go in the ancestor in TMyRealFormName?
      But it can't do anything visually, in so far as have any pre-set gui as the TMyForm is just going to visually be a blank form?


      What about DataModules (if you use them), I have a parallel layer for forms and datamodules, so generic editable, generic (but knows about some aspect of the application - my application object).....

      Delete
  4. Warren,

    I also make heavy use of form inheritance and yes, it is a hassle (luckily I'm fearless editing DFM by hand and source control is a big help there.) I'm really intrigued by your interposing classes approach. Looking forward to your post!

    ReplyDelete
  5. on the Frames - I use them extensively to build my application. for example, investments don't know about their "parents" so the specific investment frame is embedded in a parent that does know and links the 2 items. Address panes appear all over the place etc. I don't lay them out at design time on their parent, but leave panels or scroll boxes and graft them at runtime. They publish their size preference and it is up to the parent to decide if it is happy for it.

    Also, all editing / inserting is done in a generic form, with the same frame used to display the data in situ in the app. I'm arguing for frames, not the design time frame container.

    ReplyDelete
  6. I use assignable CONST as a way to preserve values for local (procedure/method-level) variables. Since VAR variables are destroyed on exit of a procedure/method, the only way to preserve the content of a variable across calls are either (1) A GLOBAL variable (ugh!) or (2) a LOCAL CONST variable that can be modified. Of the two evils, I prefer (2).

    ReplyDelete
    Replies
    1. I just asked Voldemort what he thinks, and he says he admires your technique. He particularly admires the way that other people who might have to maintain your code might have a sudden heart attack and die when they understand that you preferred not to make your global variables obviously mutable. I however disagree as to which is more evil. There is nothing wrong with Global variables if they must be globals, and many times the better solution would be a global singleton class that has read-only properties, and which can be initialized at startup with the correct final values.

      Delete
    2. This may have made(some)sense in Turbo Pascal, or old versions of Delphi, but not in any version with class vars...
      Also, what is wrong with simply declaring the Global just before the method that uses it? (other than scope)

      Delete
    3. Scope is precisely the reason. I don't want to "pollute" the global name space with a variable that other functions have no business knowing about. Likewise with class variables. If the variable is meant to be used ONLY within a specific function, then it should only be accessible from there...

      Delete
    4. > There is nothing wrong with Global variables if they must be globals

      But that's the point - they *mustn't* be globals. They *mustn't* be accesible outside the function. Show me a way to (at compile time) guarantee that, and I'll switch to it :-).

      Delete
    5. Also, Gerry... How would a class var be a solution in a PROCEDURE (not a method) ?

      Delete
  7. I recently wanted to replace some TBX controls on a base form with devExpress equivalents to make our themeing more consistent.
    Of course all of the sub-classed forms had references to "StatusBar, MenuBar" etc, it took some heavy-duty regex search and replace in NotePad++ to strip these out, along with Andy's DFM Checker [http://andy.jgknet.de/blog/ide-tools/dfmcheck/]to confirm my changes.

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

    ReplyDelete
  9. Only 2,3,4 are not useful, the rest are still very useful and handy when you have to write some quick Delphi/Pascal code

    ReplyDelete
  10. Agree with everything, except frames which can be very handy.

    ReplyDelete
  11. I agree on
    - With (except on XML routines) :-)
    - DFM Inheritance
    - Frames
    - Visual Binding (Except for small – “show Customer a small sample thing”)

    But
    - Untyped Parameters are needed… Sometimes you have to decide your own in a procedure what type you get or want to return.
    - Classic File I/O is using the same stuff internal as the Stream approach, but with less overhead.
    - Pointer: Dealing with Pointer is not so type-binded as dealing with classes, but if you want thing very fast there is no other structure to handle things. Just be careful with pointers.
    - Shortstring may be from old days, but combineing data in a record with shortstrings and other stuff is the fastest construction to write data to a socket, file, shared memory. Just one move. Dealing with referenced strings always take longer.
    - Assignable consts, are vars with lazy init. Yes you could write Var Foo : integer; Foo := 42; but where an when do you set this value. Perhaps {$J+} should be removed and a new VarConst statement should fit your needs.

    ReplyDelete
    Replies
    1. Yes you could write Var Foo : integer; Foo := 42;
      You could also write Var Foo : integer = 42; (in the global name space). But this notation won't work with local variables. If the syntax was allowed for local variables (and they would be allocated in the data segment and not the stack segment, thus preserving their (current) value across calls), then I could go away from using assignable consts (which aren't consts anyway).

      Delete
  12. Frames have their uses. I have a form where up to 9 instances of the same Frame are displayed in a Flowpanel. It works well and saved a lot of time.

    ReplyDelete
  13. What I hate are variables declared above subroutines and changes to these variables within the routines ... You can do it but you shouldn't!

    I don't like the concept of the subroutines either it makes code less testable at least in Delphi 2007 without mocking libraries. I don't know if it gets better with a newer version at that end.

    I don't like that it's allowed to write methods which don't belong to a class and can be directly called outside of the unit. Same goes for consts/vars

    And there are many more things I realy think should not or only seldom be used.

    ReplyDelete
    Replies
    1. I actually like inner functions (function inside function) but I know that it decreases readability for some people. I'd be willing to bend in anyone's direction on this one, if they'll agree with me that WITH is evil and should be removed with extreme prejudice, or whatever.

      Delete
    2. @Ludwig: I'm not sure of any problem having a methods that are independent (ie no dependancies on other variables) defined in the interface of a unit rather than having that same function defined as a method of a class. The function can be unit tested, is easier to call (no need to create and destroy a class instance) and is how many methods of the system libraries are implemented.

      Delete
    3. @Steve If your class methods don't rely on "Self" or any class variables you don't need to create that class in order to call them.
      And if it does maybe declaring those methods/variables as class methods and class variables would do the trick since class variables and class methods get initialized during unit initialization.

      Delete
    4. You are right there is no problem with having these functions if you don't use them too often or more for library like methods. But in our codebase we got over 13000 of these independent methods/variables/consts and often they are not in the unit where they should be. With a proper class arount them this wouldn't have happened (or less often) because you would think more where the method belongs. Additionally you can avoid accidentially overlapping names for methods in different units, which happens pretty fast.

      I have to say that our codebase is more than 4000000 lines of legacy code so its quite cluttered.

      (I moved my comment sry for double posting)

      Delete
  14. This comment has been removed by the author.

    ReplyDelete
  15. WITH statement very useful when you have to access several components that are in other forms

    ReplyDelete
    Replies
    1. But it hurts code readability big time especially if you have multiple forms whose components naming follows similar or same pattern.
      And code readability is what is more important than how fast can you write the code.
      What benefit is there if today you write your code in 10 minutes instead of 20 minutes when someday in the future you might have to spend about half and hour just to figure out what each part of this code does because of its poor readability?

      Delete
  16. Just subscribed via a 'feeds to email' service, I want to see an example explaining "I have even forced non-visual-inheritance using an interposing class". I agree that DFM inheritance can put you in trouble, especially when refactoring a complex UI, but it really provides benefits that you can't lose, I wonder, if you approach will allow the benefit of 'visual form inheritance', while cleaning out the hassles caused by the fact that DFM inheritance is not fully controllable due to parts of it works in a black-box.

    ReplyDelete
  17. thanks a lot for the post,keep sharing best blogs in future.

    MCX Crude Tips

    ReplyDelete