Leaderboard

The whole point of exceptions is that in a huge number of cases it is impossible to "deal with it" at the point where the error is raised. The code has been called by another function that excepts it to succeed. If it can fail, but return normally without an exception bubbling up, then you need to return information to the caller that the function has failed. And the caller may in turn be have its own caller that needs to know this. And we are right back to exception free code where errors are indicated by boolean or integer error code return values. Exceptions that bubble up the stack are only difficult to manage if you don't write your exception raising and handling code properly. For sure it is perfectly possible to write crappy exception code. But you are pointing the finger in the wrong place.

If time is a factor, these methods surely must avoid checks of init state. Yep, sometimes I'm swearing but mostly dealing with somebody's code; I'm pretty sure I'll be swearing on classes designed with your approach 😛 For me: c-tor arguments should contain crucial values that need to be set and perform some init actions so that other methods are sure the things are ready for work and they won't have to check stuff every time. You can't forget to call c-tor but you easily can forget to call separate .Init method.

Depends on the context. If you open some file for processing and it turns out to be a too large so processing it raises out of memory, you just clean up and tell to the user: "Sorry, your file is too large and you don't have enough memory." But after proper cleanup your application will be in fine condition to process some other smaller file. On the other hand, there are situations where out of memory is not recoverable. You still may be able to show message to the user there is not enough memory, but you will just have to kill the application after that.

The problem with this pattern is that most methods of the class now has to perform some kind of validation before doing their job.

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.

Wow, what a long thread and discussion for something "simple". Use the 25-year old pattern for Delphi memory management: Foo := TFoo.Create; try finally Foo.Free; end; And let the exception raising begin. Anywhere.

No, by having a destructor that can handle partly constructed objects.

Again, can somebody explain what problem is caused by raising in a constructor. We all seem fine with TFileStream doing this, so all I can see is dogma.

I don't see the point of trying not to raise in a constructor as a design goal. Raise the exception at the point where you know it needs to be raised.

It took me a bit longer than expected to get here but I believe I've finally reached the goal. The following shows VTune profiling a Delphi application, with symbol, line number and source code resolution: Download Get the source here: https://bitbucket.org/anders_melander/map2pdb/ And a precompiled exe here: https://bitbucket.org/anders_melander/map2pdb/downloads/ The source has only been tested with Delphi 10.3 - uses inline vars so it will not compile with older versions. Usage map2pdb - Copyright (c) 2021 Anders Melander Version 2.0 Parses the map file produced by Delphi and writes a PDB file. Usage: map2pdb [options] <map-filename> Options: -v Verbose output -pdb[:<output-filename>] Writes a PDB (default) -yaml[:<output-filename>] Writes an YAML file that can be used with llvm-pdbutil -bind[:<exe-filename>] Patches a Delphi compiled exe file to include a reference to the pdb file -test Works on test data. Ignores the input file Example: Configure your project linker options to output a Detailed map file. Compile the project. Execute map2pdb <map-filename> -bind Profile the application with VTune (or whatever) Known issues The -bind switch must occur after the filename contrary to the usage instructions. PDB files larger than 16Mb are not valid. This is currently by design. 64-bit PE files are not yet supported by the -bind option. As should be evident I decided not to go the DWARF route after all. After using a few days to read the DWARF specification and examine the FPC source I decided that it would be easier to leverage the PDB knowledge I had already acquired. Not that this has been easy. Even though I've been able to use the LLVM PDB implementation and Microsoft's PDB source as a reference LLVM's implementation is incomplete and buggy and the LLVM source is "modern C++" which means that it's close to unreadable in places. Microsoft's source, while written in clean C and guaranteed to be correct, doesn't compile and is poorly commented. Luckily it was nothing a few all-nighters with a disassembler and a hex editor couldn't solve. Enjoy!

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

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

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.

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". 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 )

If you want to handle the exception on site, then the second option is the wrong way to do it for several reasons. Construction of an object can always fail because of OutOfMemory, so if you wanted to handle all exceptions at that point you have failed to do so. If you are fine to bubble up that exception further on and if except part (handling the exception) cannot raise exceptions, you don't need try..finally at all.

No, you don't. If the instance is never constructed it will be nil in the destructor and you can just call Free on such instance. You only need to consider that any instance in the destructor can be nil, so if you are calling any other methods you need to check for nil before calling those in destructor.

What do you think about TFileStream.Create raising an exception if the file can be opened in the requested mode for whatever reason? I cannot see the problem with this, or indeed any constructor raising. If you are going to admit exceptions in your program at all, you must be resilient to them. Why is it any different to get the exception raised in a constructor than anywhere else.

I can't think of everything right now but the LA := TRaiseOnMinus.Create( -2 ); //<== Raise seems to me a double act. Instantiate an object and doing something which raises an exception because of the parameter. In the constructor I mostly store the argument into a class field so It will cause an exception later, thus the constructor is not affected. Similar to Uwe's solution, but without initialize, just let it fail somewhere, but not in the constructor. Also, I suspect that you are using in this example exceptions to control the app. Don't do that if you can. Make the parameter type unsigned or let it fail somewhere if the parameter is negative but do not check it in the constructor and call raise. (If you can)

Yes. All fields are zero initialized. There is no double safety if something cannot ever happen. If you have extra initialization code then such unnecessary code can obscure other important code. Keep it simple. If you are not sure what is initialized and when, it is better to check and learn than to use code just in case. It is fine to construct as many instances as you need and protect them with single try...finally. You don't need to pay attention to the order of destruction, unless there is some special reason where one instance still holds reference to other one you are destroying sooner, where you might have problem with dangling pointers. There is no functional difference, only one nil assignment more in your case - if you feel like you can more easily follow the code if multiple instances are all initialized before try..finally, that is fine. Remember, try...finally is used just for cleanup in case of exception, not for handling the exception. If exception is raised inside try..finally it will be propagated to the next exception handler block - try...except just the same it would if it is raised outside try..finally. Again, if there is exception raised in constructor, assignment to the variable will never happen. So if LA is nil and the you call LA := TRaiseOnMinus.Create(-2) in finally block LA will still be nil. Automatic cleanup for that unassigned object instance is happening in hidden code inserted by compiler. If the instance is successfully created, then automatic cleanup will not happen. Raising exceptions in constructor is not evil nor bad practice. It is perfectly fine thing to do. Whether you will raise exception in constructor or not, is matter of each particular class functionality. If you pass invalid parameter to a constructor it makes sense that such constructor raises exception, because otherwise your object is not properly functional instance, If you don't raise exception then, you would have to sprinkle your code all over inside that class and where you use it with checks for that particular invalid condition. When you have some condition that causes failure failing soon is always better than failing later on. If you have some reasons not to raise exception or there is nothing special that would require raising exception that is also fine. Constructing objects can always raise OutOfMemory so you always need to be prepared to do the cleanup anyway. Take for instance TFileStream - if constructor fails to open or create file it will raise an exception. And you can handle that failure at that point. If you don't have to handle it at that point, that means you have opened file too soon.

The advantage of moving that teardown call to the subject's destructor is that you don't need to test Assigned. If it raises an exception then you are still hosed.

For those following this thread: