FMX anti-pattern: returning nil rather than raising an exception on an invalid state or arguments

In the Delphi RTL and VCL, a method called LoadFromFile or the like will raise an exception if the file doesn’t exist or is invalid. Similarly, if an object property getter implements a lazy-loading pattern (meaning, the value of the property will only be initialised when first needed), it will raise an exception if the situation is such that nothing valid can be loaded or initialised. This is pretty basic exception programming – when an error arises, the flow of the calling code is forcibly interupted with a clear account of what call was invalid. Imagine if (say) TBitmap.LoadFromFile didn’t raise an exception when the file name passed to it didn’t refer to a valid bitmap file – calling code could go happily on its way assuming a graphic has been loaded when it hasn’t. Likewise, what if the Items property getter for TList<T> just returned nil (or equivalent) with an out of bounds index? If T is a class type, then off-by-one errors in the calling code will end up with cryptic access violations later on that might be nowhere near where the erroneous call was actually made!

Alas, but such basic understanding of exception programming is not grasped by at least one developer on the FMX team. Repeatedly across the XE2, XE3 and now (I learn) XE4 releases, methods that an experienced Delphi programmer would expect to raise an exception when nothing valid can be returned do not. As soon as one case is fixed – e.g., the TFmxObject.Children property getter was done so in XE3, albeit implicitly – another one or two are added, and frankly, it needs to stop now. The latest cases I learn are the HScrollBar and VScrollBar property getters on TScrollBox – done properly in XE3, the anti-pattern has however been introduced in XE4, causing ‘random’ access violations in TMemo (the FMX TMemo inherits from TScrollBox).

Advertisements

10 thoughts on “FMX anti-pattern: returning nil rather than raising an exception on an invalid state or arguments

  1. Years ago, I made the same mistake when I wrote our LVCL (light VCL replacement) e.g. for TList/TStringList, then I switched back to the normal exception-driven mechanism. A nightmare about consistency, you are right!

    It does make sense if you come from a C background, since you have no exception.

    It is indeed pretty disappointing to see this is “official” FMX code.

    • It’s also often encountered in many dynamic languages, though in those you usually have some form of distinction between “unassigned” and “nil/null”.

      I’ve also seen it used in the Java world, where it was used as a “workaround” for Java’s strictness about exceptions.

    • I suppose it’s two things really – whether failing to raise an exception on failure is a good idea in itself, and whether it is consistent with expected Delphi idioms. I’d say ‘no’ on both points, though you’re right to highlight the inconsistency point. If someone is expecting to have to program defensively there would be less of an issue, and things like the TMemo problem would less likely arise.

      It is indeed pretty disappointing to see this is “official” FMX code.

      Weren’t you a KSDev customer…? My ‘best’ explanation is that the original author of FMX has a personal preference against (or just ‘not for’) exceptions, which is allowing new cases to be added every release, given he is still a senior member of the FMX team (or so I assume).

  2. Initially, in the former FMX product, there was 0 exception handler, but rather a kind of “pre-contract-programming” scheme, basically at the beginning of each method you put the contract that every object used is assigned, otherwise you exit. This pattern probably must still be followed. I mean it’s designed like that…
    About the scrollbars issues…it’s possible get rid of that by using those “pre-contracts” ( I’m not a expert in design but I think some people also call this “a protector code”).

    • This pattern probably must still be followed.

      Sorry, but I completely disagree with that. What do you mean by ‘pre-contract programming’ exactly? So far as I can see, the author returns nil out of ignorance, though I’m willing to learn the method behind the madness… 😉

      • I mean that the “contract” is “to always test if an Object is valid before using it”…created by lazy-eval or not…because for example until the style is applyed, a FMX sub-object with a prop getter in the parent is nil….
        Stop to complain and protect your code…or at least try to understand how it works !

        But I recognize that if the scrollbars are created by lazy-eval there is a problem…technically in FMX a primitive is created when reading the style and then assigned to an Object pointer…So since I missed an episode (vgScene to XE2) it seems that I also missed a season…
        I still have the hope that the dozen of over-hypped-douche from DelphiFeeds and StackOverflow will ever get how FMX works…

        • I mean that the “contract” is “to always test if an Object is valid before using it”

          I’m not sure whether your arguing it isn’t, but that is an appalling approach to enforce, and shown by the stupid bugs it is causing. It’s like programming in the pre-.NET VB! Mind you, given Delphi has its very own Option Base and reference counted objects now, perhaps that’s quite apposite…

          I still have the hope that the dozen of over-hypped-douche from DelphiFeeds and StackOverflow will ever get how FMX works…

          If that’s directed at me, I’m sorry, but you don’t know what you are talking about. Avoiding exceptions because you don’t know how they work isn’t a clever piece of framework design – it’s a jejune mistake.

  3. Pingback: Te Waka o Pascal · iOS “Support” Prior to XE4 ?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s