views:

281

answers:

6

I am looking at some code (Delphi 7) with following check is at the top of every method call for a specific object:

if not Assigned(self) then
  raise Exception.CreateRes(@sAbstractError);

  { Real code for this method}

I guess that this would prevent me from trying to call a method on a null object pointer. But I would get an exception as soon as I tried to access member data in that case anyway, right?

Is this some type of standard that I have never seen before? The object in question derives from TPersistent.

+3  A: 

There are access violation scenarios that can lead to code running in memory where Self is nil. Some programmers, instead of solving the real problem, try to bypass is using such assertions, to prevent corruptions of sorts. I would check very well if that Exception ever raises in runtime, if so - you've got buggy code under your hands.

You can read the following article about the subject: When Self in Nil... You Know You are in Trouble.

Another possibility is related to events, you can read more here, with plenty of examples for such tests, and corresponding explanations: Multicast Events - Part 2.

Roee Adler
I wouldn't say this example code bypasses the real problem at all. On the contrary, it highlights the problem explicitly.
Rob Kennedy
+7  A: 

A clear error complaining about a nil pointer is better than an access violation that doesn't say how it happened.

Loren Pechtel
+1 for catching a nil pointer rather than a *possible* access violation.
Ian Boyd
So are you saying you code all of your objects this way?
Mark Elder
It's very hard for a nil pointer dereference not to go boom. You don't own the memory down there.
Loren Pechtel
A: 

And then there's always the possibility that the code would not crash when running with nil Self. Example - if it is not accessing any fields of the owner object. In that case, this test would catch the problem that would otherwise be undetected.

Still, that's taking defensive programming to the extreme. I never (OK, almost never - only when I'm hunting wabbits ... errr ... squashing bugs) do that.

gabr
Even though i've done it - it's poor form to use a non-class method as if it were a class method.
Ian Boyd
A: 

Seems the original intention for that method is that it become a abstract method.

Fabricio Araujo
+2  A: 

You can call a instance method on a null pointer although it's not the sort of thing you want to do deliberately. When that happens, execution continues along quite happily until it needs to access the instance data and then it all goes bang.

In that case, checking for nil would alert you at the top of the procedure so you could do something different such as logging a stack trace. Or you could put a break point on the raise line so you can dive in and see what is happening.

Ie, it's something that I might do if I had a specific bug I was trying to track down where a nil reference was being used.

Doing it regularly strikes me as a code smell.

SeanX
+1  A: 

This is an occasion that should definitely lead to a crash (i.e. a OS exception like "read from address 0x00000000"). Throwing a language exception is simply redundant (and misusing EAbstractError here doesn't make it better).

Checking for valid input parameters isn't a feasible task in an unsafe language, as you will never gain certainity, and thus your handling of invalid parameters will never be consistent. (Why do you throw an exception when a nil pointer is passed, but not for 0x00000001, which is just as invalid?). For a more technical discussion of this question, read Larry Osterman's blog about why not to check for valid pointers.

One exception should be noted: in Delphi, the Free method may be called on a nil pointer, which of course requires that kind of checking.

Moritz Beutel