20

SITUATION

I am going to write a class and the constructor is a custom one that I have made because I need to initialize some values. This is the code I've written so far:

type
 TCombinatorio = class(TObject)
  private
   valN, valK: integer;
   result: double;
  public
   property K: integer read valK;
   property N: integer read valN;
   constructor Create(valN: integer; valK: integer);
 end;

constructor TCombinatorio.Create(valN: Integer; valK: Integer);
begin
  inherited Create;
   Self.valN := valN;
   Self.valK := valK;

  if ((valN < 0) or (valK < 0)) then
   begin
    raise Exception.Create('N and K must be >= 0');
   end;

end;

Since I am going to do some math calculations, I need to avoid negative numbers.

QUESTION

Can I raise an exception in the constructor in that way? I am running the code in this way:

procedure TForm1.Button1Click(Sender: TObject);
var a: TCombinatorio; 
    b: string;   
begin

 a := TCombinatorio.Create(5,-2);

 try
  //some code
 finally
  a.Free; 
 end;

end;

As you can see here I have wrong parameters for my constructor, since the second is negative. I also cannot understand (according with the code of my constructor) if the a.Free inside the finally is really needed because when the constructor raises the exception, the destructor is called.

I thought to include the a := TCombinatorio.Create(5,-2); inside the try-finally block to avoid the problem but I am not sure. What do you think?

Improve this question
0

4 Answers 4

Reset to default
26

Your code is absolutely fine and correct. Raising exceptions from constructors is perfectly respectable. As you know the destructor is called.

You ask about this code:

a := TCombinatorio.Create(5,-2);
try
  //some code
finally
  a.Free; 
end;

You are worried that Free will be called after the object has already been destroyed. That cannot happen. If an exception is raised in the constructor then it propagates up the call stack. That happens before the try block begins and so the finally block does not execute. Indeed the assignment to a does not happen.

Moving the creation inside the try would be disastrous and is in fact an incredibly common mistake. Suppose you did that:

// WARNING THIS CODE IS DEFECTIVE 
try
  a := TCombinatorio.Create(5,-2);
  //some code
finally
  a.Free; 
end;

Now if an exception is raised then Free is called but on what? The variable a is not initialized. Even if it was, which it isn't, that would still be a double free.

Improve this answer
Sign up to request clarification or add additional context in comments.

18 Comments

@kludg - If you know at construction time that an instance is going to be unusable, the constructor is the best place to raise an exception imo. The alternative is raising an exception when the instance gets used which could lead to interesting debugging sessions to know where the error originates. That said, in this specific case, OP should switch to unsigned integers.
@kludg: I totally disagree. Mind that class fields of reference type (like TObject) are always initialized with nil, unlike local variables. Call myAggregatedObject.Free() in your destructor and you're fine. You should, however, refrain from calling virtual methods in your constructor, but that's an entirely different story and not only applies to Delphi.
@serg Raising exceptions in destructors is wrong. But it is explicitly designed for and supported in constructors. Don't you write your destructors to cope with partial construction?
@LievenKeersmaekers. The class can handle negative numbers. It says they are not allowed. If you make its constructor only handle unsigned the when you try to create with a normal integer (as you inevitably will) you need to check whether that number is negative. The compiler can, by default, only help with a warning and you have no way of knowing at compile time what value the integer will take. It might always be valid. it might not. The only alternative to the proposed approach would be to put the testing outside each instance of code creating the object, now and forever.
@Dsm - If you have Range Check Errors enabled (as I believe you should), you'll get an error trying to pass a negative value. Testing and raising an error yourself is only overhead. If one compiles his program without Range Check Errors, all bets are off but in that case, most likely there are plenty of other subtle bugs in the program waiting to be discovered.
|
4

OK, first you can raise an exception in the constructor, and yes it does call the destructor as a consequence. The code you show is fine. But I think you misunderstand what your code does. And to put the constructor inside a try finally block would be wrong. The point I think that you are missing is that if your constructor fails the try...finally block never gets executed and so the free is not executed. You should not call free if the constructor does not succeed, which is why you should not put the constructor inside the try...finally block.

Improve this answer

Comments

4

First of all I would say that you cannot avoid exceptions in constructors so it cannot be an anti-pattern. If you check Delphi source code you will find number of places where exception is raised in constructor. For example

constructor TCustomForm.Create(AOwner: TComponent);
begin
  // ... skipped some lines
        if not InitInheritedComponent(Self, TForm) then
          raise EResNotFound.CreateFmt(SResNotFound, [ClassName]);

The only thing you should know is that Delphi will automatically call the destructor if an exception escapes from the constructor. Actually it means that your destructor may be executed on a partially constructed object and it is your responsibility to write destructor properly. See TObject.Destroy documentation, and pay your special attention to the below quote:

Note: If an exception escapes from the constructor, the destructor is called to destroy the partially constructed object instance that failed to initialize completely. Therefore, destructors should check that allocated resources such as handles were actually allocated before trying to release them, since their value might be zero.

PS In general you should assume that each line of code may raise an exception, but please do not be a paranoiac ;)

Improve this answer

2 Comments

I would add that if you do so (raise exceptions in ctor), you'll have to deal with testing wether your object is created or not before each call in your client application! Because if exception occurs --> destructor called automatically --> object = nil --> object.DoSomething will result in Acces Violation
Normally there is no need to test it because your object.DoSomething code should not be executed because of the exception. I would appreciate if you could post a code sample where this check is required. I would assume that either you have handled the constructor exception manually or your code can be called from the destructor.
0

In that case I am usual add methods which check the data:

.. = class
function DataValid : boolean;
...
end;

Lots of benefit:

  1. no exception in constructor. It simple copies its parameters to in-class fields.
  2. special destructor for partially created class not needed.
  3. simplicity of code.
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.