1

I think this is a "best practice" category question:

I have a custom control - some kind of grid that holds some panels. One of the panels is the current active panel (the last one clicked).

TMyGrid = class (TSomeKindOfGrid)
  published
    property CurrentPanel: TPanel read getCurPanel write setCurPanel;
end;

My question is: if at a certain point someone asks for the CurrentPanel and the grid is empty, should getCurPanel return a NIL or should it raise an exception?

  • If getCurPanel returns NIL, then I MUST check for NIL everywhere/every-time I call CurrentPanel. There is also a possibility that the caller forgets to check for NIL. Well, nothing "bad" will happen since it will try to access a NIL object. The program will nicely crash right there. And I get to get a stack trace.
  • If I raise an exception in getCurPanel, I only do the check in one place (yes, I'm lazy).
LU RD
  • 34,438
  • 5
  • 88
  • 296
Gabriel
  • 20,797
  • 27
  • 159
  • 293
  • 1
    There's no difference. Everytime/everywhere you access a non existing panel there's an exception, either you raise it yourself or the runtime. If you want to avoid it you have to check first, in either case. – Sertac Akyuz Apr 28 '19 at 16:05
  • @SertacAkyuz there is no *guarantee* that accessing a nil pointer will raise an exception. It is undefined behavior. Anything could happen. An exception *might* be raised, or you *might* just read garbage, or you *might* trash memory, or ... – Remy Lebeau Apr 28 '19 at 17:36
  • or ... demons might fly out of your nose. – Rudy Velthuis Apr 29 '19 at 06:22
  • @RemyLebeau- hi. can you please comment on this? https://stackoverflow.com/questions/55898595/there-is-guarantee-that-accessing-a-nil-pointer-will-raise-an-exception – Gabriel Apr 29 '19 at 07:15

1 Answers1

5

If you return nil, you give the user the opportunity to check the return value and skip anything he or she intended to do with the current panel:

panel := XYZ.currentPanel;
if Assigned(panel) and (panel.Index = 17) then
begin

The above code runs without any unnecessary interruptions.

If you immediately raise an exception, you don't give the user the opportunity to find out if there is a current panel at all. In other words, raising an exception would be premature. The same code as above will blow up.

But I admit that this is my personal preference (and probably that of many, but not all). This is a matter of opinion.

But instead of returning a nil, you could also expose a PanelCount property. If people have something like that to check, you can just as well raise if someone tries to access a panel if count is zero. Then it is not premature.

As you can see, there are several ways to do this.

Note

As SilverWarrior correctly noticed in a comment, currentPanel is a published property which will eventually appear in an Object Inspector. That can handle a property returning nil, but not necessarily a property that throws an exception.

So: the best advice is to return nil.

Rudy Velthuis
  • 28,387
  • 5
  • 46
  • 94
  • 1
    I agree that exceptions are not the way to go, but it's not true (or at least not cut and dried) that an exception gives the consumer "no opportunity" to detect and react to their being no panel. It just makes it harder and inconvenient, requiring a try/except handler and a new, specific exception type that specifically identifies "No current panel" as opposed to some other exception type which could theoretically be raised by other code enclosed in the same try/except. Ugly, messy and more complicated are the reasons to avoid exceptions (in this case). – Deltics Apr 28 '19 at 20:55
  • Sure, if you catch the exception, you also know that there was no panel and **that is probably why the question arose**. But that is IMO not a proper way to handle this. – Rudy Velthuis Apr 29 '19 at 06:20
  • 2
    There is another reason why OP shouldn't raise exception in this case. If you check code that OP provided you can see taht the declared his property as published. This means that most likely he intents to have this property visible in object inspector during design time. So while Object Inspector is made to properly handle properties that retutn nil as result I seriosly doubt it would handle any exceptions being raised. In fact by rasing exception in published property you might even crash Object Inspector entirely. – SilverWarior Apr 29 '19 at 06:56
  • @SilverWarior - Yes. CurrentPanel is a published property, to be used at design time. Therefore, your answer is sound. – Gabriel Apr 29 '19 at 07:11
  • Of course there could also a function TryGetCurrentPanel(out Panel: TPanel): boolean which gives the caller a simple way to check if Panel is valid before using it. – dummzeuch Apr 30 '19 at 08:45
  • It is not uncommon to have a property like ˋCurrentXYZˋ or ˋActiveXYZˋ and such a property should not „throw“. – Rudy Velthuis Apr 30 '19 at 11:06