BestPractices: To raise, or not to raise ... an Exception in a class constructor

[Rollo62 536]

1 hour ago, Wagner Landgraf said:

Wow, what a long thread and discussion for something "simple". Use the 25-year old pattern for Delphi memory management:

Right, I'm also wondering how such a "simple" question touches so many basic language concepts.

 

But to clarify the original question again:

It was not about if raising exception in Create is possible and how to do that and catch it, the question was:   

"Shall I raise exceptions on purpose, or better not".

 

1 hour ago, Darian Miller said:

Also consider passing items into a constructor rather than creating items within it to increase testability.  

Yes, this is exactly what I am looking for, especially here I should raise exceptions if something is wrong with these parameters.

 

Thanks to all, who brought so many different aspects into this discussion.

 

I came already to the conclusion:

Raising on purpose in Create() is fine, especially to guard against missing or faulty injected parameters.

 

Everything else is just an intermediate state to the final, perfect design, but there is no real reason to keep such intermediates.

( only I'm not so sure if I am ready to "finalize" all my classes in time  )

 

[Attila Kovacs 629]

2 hours ago, Wagner Landgraf said:

Wow, what a long thread and discussion for something "simple". Use the 25-year old pattern for Delphi memory management:

 

Can't see a damn question about memory management. Maybe I'm reading a different thread.

[Attila Kovacs 629]

21 minutes ago, Rollo62 said:

Raising on purpose in Create() is fine

It is.

 

21 minutes ago, Rollo62 said:

especially to guard against missing or faulty injected parameters.

like if Parameter1 = null then raise...?

What if that object is freed before you will use that in some of the methods?

What is the advantage of this early-validation? It's just flow control in this case.

[Rollo62 536]

20 minutes ago, Attila Kovacs said:

like if Parameter1 = null then raise...?

What if that object is freed before you will use that in some of the methods?

That external parameter is not in the domain of the class, so I have to take care outside, to keep everything clean.

 

Or do you mean issues with some kind of delayed access, in anonymous procs, events or threads ?

When the object might be released, while the event proc might still try to touch it.

I think this could has to be guarded by other means, its not related to the constructor exceptions.

 

20 minutes ago, Attila Kovacs said:

Whais the advantage of this early-validation? It's just flow control in this case.

I would not need Guards on every internal operation, to ensure all parameters are OK.

I would hardly see any strange sideeffects later on, when an invalid parameter causes issues only under rare conditions, the creation fails gracefully immediately before that happens.

 

[Wagner Landgraf 43]

1 hour ago, Attila Kovacs said:

Can't see a damn question about memory management. Maybe I'm reading a different thread.

I see lots of discussions about how to properly structure try..finally statements and how it behaves depending if a constructor raises an exception or not; if the object should be created inside or outside the try..finally, if the destructor should have a try..finally inside it, etc.

Maybe indeed it's a different one. This is the one I'm reading: https://en.delphipraxis.net/topic/6149-bestpractices-to-raise-or-not-to-raise-an-exception-in-a-class-constructor

 

Edited January 12, 2022 by Wagner Landgraf

[Wagner Landgraf 43]

1 hour ago, Rollo62 said:

I came already to the conclusion:

Raising on purpose in Create() is fine, especially to guard against missing or faulty injected parameters.

Amen to that.

Edited January 12, 2022 by Wagner Landgraf

[Attila Kovacs 629]

49 minutes ago, Rollo62 said:

I would not need Guards on every internal operation, to ensure all parameters are OK.

 

So you are expecting and handling exceptions from the constructor but not expecting and not handling exceptions from any further method calls?

Interesting.

 

[Dalija Prasnikar 1396]

2 hours ago, Attila Kovacs said:

Can't see a damn question about memory management. Maybe I'm reading a different thread.

try...finally implies cleanup which in Delphi terms implies memory management 

 

Maybe memory management is not main topic, but it is certainly related.

[Dalija Prasnikar 1396]

32 minutes ago, Attila Kovacs said:

So you are expecting and handling exceptions from the constructor but not expecting and not handling exceptions from any further method calls?

Interesting.

The way I am reading that is: if you pass parameters in constructor to initialize fields and you validate them there, raising exception if they are not valid, then you can avoid checking those fields when you are using them in other places because you know they cannot hold invalid values.

[Attila Kovacs 629]

Just now, Dalija Prasnikar said:

The way I am reading that is: if you pass parameters in constructor to initialize fields and you validate them there, raising exception if they are not valid, then you can avoid checking those fields when you are using them in other places because you know they cannot hold invalid values.

Must be a very rare occasion where the parameter type itself does not cover the validity and the validity would not change with time, as far as I can think of. In this case of course a validation is not avoidable. However, I still think, that that is not the case behind the problem because it should be such rare, that he would not brought it up. I suspect over- and early-validation. At least, my advice was, not to do them if not neccessary.

[Attila Kovacs 629]

14 minutes ago, Dalija Prasnikar said:

try...finally implies cleanup which in Delphi terms implies memory management

he has no problems with memory management, he even made it clear in just a couple of replies above so I have no idea what you trying to prove here

[Wagner Landgraf 43]

12 minutes ago, Attila Kovacs said:

Must be a very rare occasion where the parameter type itself does not cover the validity and the validity would not change with time, as far as I can think of. In this case of course a validation is not avoidable. However, I still think, that that is not the case behind the problem because it should be such rare, that he would not brought it up. I suspect over- and early-validation. At least, my advice was, not to do them if not neccessary.

A very common case is when you inject objects and interfaces in the constructor. At the very least you would want to check if they are not nil.

[Wagner Landgraf 43]

9 minutes ago, Attila Kovacs said:

he has no problems with memory management, he even made it clear in just a couple of replies above so I have no idea what you trying to prove here

He was not the only one posting here. 

[Dalija Prasnikar 1396]

8 minutes ago, Attila Kovacs said:

he has no problems with memory management, he even made it clear in just a couple of replies above so I have no idea what you trying to prove here

I am talking about your comment to  @Wagner Landgraf where he mentioned memory management and you are saying this thread is not about memory management. 

 

I am not trying to prove anything, just wanted to say that memory management is indirectly involved in this conversation, and Wagner didn't post in the wrong thread.

[Attila Kovacs 629]

4 minutes ago, Wagner Landgraf said:

A very common case is when you inject objects and interfaces in the constructor. At the very least you would want to check if they are not nil.

I would eventually, if I'm using them in the constructor. Eventually, as I'm not sure that an exception("I'm dumb") is better than an AV.

If I'm not using them in the constructor, there is no reason to validate them.

Also, it can be still trash because it's not nil.

 

[Attila Kovacs 629]

8 minutes ago, Wagner Landgraf said:

He was not the only one posting here. 

ok, you are right, I missed a lot from the beginning, my apologies

[David Heffernan 2345]

8 minutes ago, Wagner Landgraf said:

At the very least you would want to check if they are not nil.

I don't really see the value of this, if the input is contracted to be assigned. That makes it a programming error, your tests will AV and you will fix it. No need to clutter the code with if not Assigned tests. 

[Wagner Landgraf 43]

1 minute ago, Attila Kovacs said:

I would eventually, if I'm using them in the constructor. Eventually, as I'm not sure that an exception("I'm dumb") is better than an AV.

If I'm not using them in the constructor, there is no reason to validate them.

Also, it can be still trash because it's not nil.

 

If the interface is required for your whole class to properly work, why you wouldn't want to check it in the constructor? Why allowing a instance to be created in an "invalid" state? You would have to check if the interface is nil everywhere in your code and for no use, since your class won't work without it.

[Wagner Landgraf 43]

1 minute ago, David Heffernan said:

I don't really see the value of this, if the input is contracted to be assigned. That makes it a programming error, your tests will AV and you will fix it. No need to clutter the code with if not Assigned tests. 

Fair enough, it was more an example to show that relying on the parameter type is sometimes not enough. But as a library writer I don't think it's bad to give a better error message to the user of my classes (whose code I have no control of) telling him what he did wrong, instead of raising a cryptic AV and waiting him to realize what he did wrong or even blame the framework itself.

[David Heffernan 2345]

2 minutes ago, Wagner Landgraf said:

why you wouldn't want to check it in the constructor? 

Because it clutters the code and you can find such bugs other ways which don't clutter the code. 

[David Heffernan 2345]

1 minute ago, Wagner Landgraf said:

But as a library writer I don't think it's bad to give a better error message to the user of my classes (whose code I have no control of) telling him what he did wrong, instead of raising a cryptic AV and waiting him to realize what he did wrong or even blame the framework itself.

Oh for a library writer then yes, the calculations are different and there is much more justification for this

[Attila Kovacs 629]

But as a Library-Writer, this validation is a Plus, and not a "guard" for the rest of the code 😉

[Fr0sT.Brutal 900]

17 hours ago, Darian Miller said:

Also consider passing items into a constructor rather than creating items within it to increase testability.  

Yep, what could be better than explicitly creating a canvas and icon for every created form 😄

 

[Uwe Raabe 2057]

I am a bit with @David Heffernan here: As long as I am the only one using the classes or even the team is, I tend to avoid checks for parameters or correct usage - despite documenting it in the comments (like XMLDOC) of course. For myself and the team I expect to take care to do it right, otherwise running the test suite will (should) surface that early enough.

 

In contrary, writing library code exposed to arbitrary users requires a bit more defensive programming, at least for the interface part the user is accessing directly.

 

[Lars Fosdal 1792]

Sliding into off-topic, but I avoid parameters to constructors. 

The primary reason is that parameterless constructors are a good fit for polymorphic constructs, as well as instantiations for generics. 
For configurations, I often give the instance an interface to fetch its own config from.  DI FTW.