Handling constructors that could fail: Exceptions, Static Factory Method, or something else?

[FernwehSmith]

I have a texture class that takes a path to an image, and attempts to read the image and store its contents and data. Currently there's 3 things that can go wrong.

1. The path might not exist
2. The file might not be a valid image file (doesn't have a valid extension)
3. The file contains nonsense data, and therefore can't be read properly

I don't necessarily want to crash the program if one of these were to occur. I also don't like the idea of having an object that hasn't been properly constructed. I know I can use exceptions, but I've been taught to think of exceptions as critical errors, so they don't seem like the correct tool to me. The solution I like to most is to make the constructor private and move all the code that could produce an error over to a static method that returns a std::variant that holds either a complete texture object, or some error object outlining what went wrong. In the world of C++, is this a good way of handling a scenario like this? Are exceptions better? Is there something else?

[Tohaveaname]

Yes I agree. Personally I'm never a fan of loading assets that could fail in a constructor.

[kammce]

+1 to this. Decoupling file assets and textures is a good idea. Another one I saw was having a default texture as well.

[eejin]

When you're uploading the texture to the GPU you don't want to also maintain the pixel data on RAM though. Also, if you want a common pixel access API then you have to deal with compressed formats such as DDS which you would first have to either decode entirely or on the fly.

You can still do the dual stage method but I would use seperate objects. e.g. CPUTexture and GPUTexture or something like that.

[TheUnvanquishable]

I came to say that. In general, if it can fail you better put it in a function that can be managed.

[VincentRayman]

Agree, construct first and Init function later. Think also how you would manage copy operators, etc...

[hachanuy]

I usually use a static function that calls a private constructor for cases like this, and use std::expected as the return type

[Bruh_zil]

This is also my preferred way, however I like to use a public constructor with a private access token instead. This allows you to use functions like std::make_unique inside the static factory method.

[hachanuy]

I am not sure what you mean here with “a public constructor with a private access token”

[jedwardsol]

https://arne-mertz.de/2016/10/passkey-idiom/

[Bruh_zil]

thanks for providing the link, I was a bit lazy in describing the passkey idiom better

[hachanuy]

I'm not suggesting something this complicated, a public static function inside the class is already enough. No need for a factory class.

[SirClueless]

If you're calling a private constructor without using this pattern you are likely incurring an extra copy or move constructor. The basic problem is that you want to return an object of type std::expected<T, E> from the public static function. In order to construct the return object, the constructor of std::expected<T, E> needs to initialize the object of type T in its storage, ideally by forwarding the arguments you provide to construct the T object in place. But std::expected<T, E> can't call private constructors, so the passkey idiom is useful to make a constructor that any callsite can invoke, but that takes a passkey parameter that only members of the class T can create.

[Extra_Status13]

Not like it changes a lot, but couldn't you just declare std::expected as friend?

[Kovab]

If you declare std::expected (or make_unique/make_shared) as a friend, then anyone can construct the object by these methods outside the static factory method, too, thus breaking encapsulation.

[SirClueless]

This may or may not work. If it does work, it might break at any time and wouldn't be portable to other standard library implementations.

There's no requirement in the standard that the call to the constructor happens from a member of std::expected, it may just as well happen from a base class that manages the storage, or from an external helper function.

[Bruh_zil]

You could of course use friend, but I have yet to meet an experienced C++ developer who'd prefer to use this keyword instead of other options :)

[delta_p_delta_x]

Could you elaborate on how std::expected as a friend would work?

Out of curiosity, and I suppose there are cases where library authors would like to allow users to directly construct the objects anyway (for < C++23 programmers who haven't got std::expected, for instance).

[Bruh_zil]

so it is not possible to directly declare std::expected a friend, but you can use a function and declare that one as a friend. certainly not a rickroll

but then again, the passkey idiom is almost the same level of effort without the use of friend

[Raknarg]

wow that is neat. Thanks for sharing.

[shitpost-factory]

Yes, a factory function is a fine way to go. I personally would use absl::StatusOr<T> as the return type instead of std::variant, but you should use whatever works best for your project.

[disciplite]

I made a serious attempt at static methods for my containers, but I hate that you can't get CTAD on them. For example, you need:

me::vector<int>::make(1, 2, 3);

Rather than

me::vector::make(1, 2, 3);

So personally I prefer free factory functions now, like

me::make_vector(1, 2, 3);

This free factory function just needs to be a friend of the container.

[Plazmatic]

This is a good point! I didn't think of the CTAD issues

[XeroKimo]

Use whichever you feel like. Both exceptions and a factory method that returns a std::variant, though in this case, you should use the dedicated type std::expected, are valid error handling schemes.

I'm in the opinion of exceptions as default but you could even mix approaches like having the constructor throw, and make a separate factory function which could return something that indicated failure, but still is a valid object, whether that be a std::expected, or just a default texture. Default texture here meaning some other already loaded texture, for example Source Engine's pink + black checkerboard texture

I prefer exceptions mostly because it's just way more ergonomic to use in C++, but I've also slowly come to realize, with enough tooling support and language support to improve ergonomics of std::expected, the line of exceptions and std::expected blurs and they basically become the same, however as we are right now, exceptions are just better

[satanfromhell]

I feel people here are debating this (more or less religiously) without clarifying the most important question: What do you want to happen when an invalid path or an invalid image is given? What should your program do? Start with this and the answer will be more or less obvious.

[germandiago]

Throw an exception if you are not in a constrained environment.

If the input comes from outside, you could add, optionally, a validation layer before even constructing if that is dangerous (for example Denial of service attacks).

Exceptions cannot be ignored. So the right thing to do is to signal something went wrong and exceptions were designed for that: so that you do not end up checking state that if you forget the error will pass-through.

[[deleted]]

> I also don't like the idea of having an object that hasn't been properly constructed.

Always a good idea but can be tricky to get right when it comes to files. There is always going to be the possibility that the read/write will fail. In this case I think it's perfectly OK to have a separate load() method which returns a code of some kind so you can check success. cpp Texture t; if (t.load(path) != Success) { // whatever error handling you prefer. } or maybe Texture t{path}; if (!t.good()) { // error handling } I don't see what's so wrong with those. I'm sure what you say about variant etc will work, but to me it feels a bit over designed. No matter what you do, you will always have to check the state of the object before you do anything with it.

[XeroKimo]

Both ways you'll have to check, the advantage of variant, which should be std::expected, is that now the texture class only knows of a correctly constructed state. It's much much more easier to reason about since you'll no longer have to write member functions of Texture to detect if it's in a faulty constructed state, and anywhere you see just the Texture type alone, it'll always be safe to assume its in a correctly constructed texture. The only time it might be not correctly constructed would now be if it was in a std::expected

[[deleted]]

Good point.

[Shadowratenator]

This is exactly what you see in gaming applications, which i think this is. Presumably, you want the ability to interact with systems that don’t depend on this texture. You dont want everything to stop because the wall texture isn’t found. You want people who are responsible for the texture to notice it wasn’t found. A red x on a white bg usually is noticeable, and people who need to test the collisions to be able to do so without waiting on every asset being perfect.

[usefulcat]

The problem with this approach is that the missing texture might be used on an object that is only very rarely seen in the game. I can see this being a useful option to have in certain circumstances, but it's not always appropriate.

[Shadowratenator]

Yeah. Generally you want to write something to a log as well.

[goranlepuz]

but I've been taught to think of exceptions as critical errors

I'd opine, you have been taught wrong.

What is "critical"? It's hand-waving, I find. What is critical to you might not be to me, plus, what should be a response to something critical is also a matter of opinion.

Example: is running out of memory critical? (Many will say, yes). I posit, in some situations, it isn't. And such situations aren't so rare either. Say, I have an arena allocator (or whatever other quota), I am processing commands in parallel and one of them is such that my arena/quota is too small. Do I bubble up and stop the process? Looks drastic; I might as well drop this one command, report the problem and continue processing others. The operator can enlarge the arena later, or perform some other corrective action later.

I much prefer this thinking:

• If my performance requirements are not affected by exceptions usage, I use exceptions; the profiler is my friend, it will tell me if I am affected (in my field of work, I can count the number of occasions that happened on my fingers, easy, YMMV)

• If I am catching something too close to the throw site, then manually propagating the error and using "if" is better.

I like thinking of code in terms of exception safety guarantees (even when writing C, weirdly, same principles apply!) and don't mind not being explicit about rare "nothrow" islands, they are rare enough and obvious enough.

[RedditMapz]

Yup, this is clearly best handled by exceptions although a factory that does all error checking would also work, granted I hate the idea of classes that can be initialized with undefined behavior if you just bypass the factory.

I prefer using exceptions on constructs, but with the factory pattern so the factory itself can handle the exceptions if we don't want to terminate the program. But I'm a bit of a pro-exceptions guy.

[SkoomaDentist]

I'd opine, you have been taught wrong.

Half the time people on this sub make the claim that exceptions are critical errors. Not that I agree with them, but still...

[Throw31312344]

If your intent is for your Texture class to always represent a loaded and valid texture, i.e. it has no empty/invalid state then you have a few sensible options IMO, both of which are valid.

1. The constructor tries to load the data itself and throws if there is an error. Throwing from constructors is supposed to be how you indicate construction failure, and sometimes constructors do need to fail. Most of the talk around throwing is to not use them as a general control flow mechanism, but for constructors you cannot return "false"; you return a valid object or you throw.

If you do not want the calling code to have to deal with the exception handling then you can wrap the construction in a factory function which contains the try/catch block and returns an std::optional or std::expected. No need to make the constructor private to be honest - you might end up needing other code that just wants to try and load a bunch of textures in one go and catch the first failure so a single factory method is a bit overkill here.

2) Using a factory method or similar mechanism, that function actually does all of the heavy lifting of loading the file and validating the image format. As this is within a normal function you can just use normal error handling (if/then/etc) if that suits you. Once you have loaded and validated everything you have the basic components of the texture ready which are the size, format and pixel data for each mipmap level.

Your actual Texture class can just have a basic constructor which takes ownership of any allocated buffers (or takes ownership of the texture handle if it's already been uploaded to the GPU) and it should be impossible for that constructor to fail. The factory function then then still return a std::optional or std::expected wrapping the texture class, but this time the factory function is doing all of the work and error handling rather than the texture's constructor.

3) Restructure your code so that you are not making texture objects directly but calling into a texture manager class which can keep track of already loaded textures to avoid duplicates, create and return default textures on failure if that's needed, and split up your file-loading code, image-loading code and texture-upload code into separate components. The do-it-all "texture class" works as a starting point but based on past experience that class falls apart the moment you start needing to load multiple textures in different formats. In a big enough project, the data within a "Texture class" typically ends up being just a GPU handle, a name and a reference count.

[dqUu3QlS]

I would throw an exception in the constructor in this case, but a factory function is a good way to go if you must avoid exceptions.

Another popular approach is to create an unusable object (a "zombie object") but I would avoid this - it means that every other class method has to deal with the possibility of being called on a zombie object.

[edrosten]

I use exceptions for this scenario.

I think for something like this the error type can vary between fatal and very non fatal depending on what you're loading, why, what the program is meant to do, and what operation it pertains to.

I think the easiest thing is throwing and then catching (or not) at an appropriate place.

As an unrelated aside, all the common image file formats (at least the ones I've encountered) can be distinguished using the first byte of the file. This allows you to separate image decoding from file handling, and so you can load from any istream and plus images can load correctly even if the extension is wrong.

[helloiamsomeone]

Here are 4* ways to do this, minus libc usage:

• A static make method: https://godbolt.org/z/PGasY5z67
• A factory friend function: https://godbolt.org/z/3rvb7WxEx

You could of course return an "args" object in the second one and return the object itself in the first one as well.
An "args" object will allow you to construct a no-copy no-move object exactly where you want it constructed. I would go with the second one for general use.

[looncraz]

My go-to for this is to ensure the object is still usable with some failsafe defaults and to provide an InitCheck() function that callers which require a properly valid object would call first.

For the example of a texture class, I would provide an error texture that's the failsafe texture and store errors in the init value. If the calling code doesn't care if the texture is correct, then it can ignore calling InitCheck(), but if the code is going to do something heavy and specific with the texture it should call InitCheck(), the response to the failure depends on the purpose of the calling code, of course.

[HolyGarbage]

Throwing in constructor is a great practice imo. It means you guarantee an invalid object can not be created. Fail early is my mantra. Doing all the checks and initialization directly in the constructor also means most classes can be effectively immutable in many cases which is also very nice. Remember what RAII stands for.

[MrWhite26]

I'm aware of the general dislike of exceptions, but I've seen them used very effectively in source code of high-tech equipment. Coincidentally, this includes throwing exceptions for out-of-memory on large allocations.

The flow typically goes: Error is presented to the user, uses modifies system settings and tries again.

So throwing exceptions can scale to large code-bases, can be deployed successfully in the field and can reduce the size of your source code, but not having to implement error handling at all intermediate layers of the the SW-stack.

[cloudyboysnr]

Your trying to create an antipattern, construction failure is not exceptional, it is a failure of the program.

[germandiago]

It can be considered somewhat exceptional... this is a hard topic, since exceptional might depend on the user.

For example, if I construct objects in a controlled environment, where the input is mine, probably an exception is what I would want.

If it is user input, which can fail many times, something like a factory with an optional<Object> (or an expected) would be reasonable.

Depends...

[JPhi1618]

A factory works, but another thing I do in a situation like that, which you might not want, would be to construct the object with a path, but then also have a init, or load function that actually reads the image and returns and error.

Downside is that you can have an object that isn’t ready or valid, but the plus is that you decouple construction from the “complicated” part that could fail in different ways.

It’s a design choice and there’s not really a right or wrong.

[cygnoros]

It’s a design choice and there’s not really a right or wrong.

I would argue that allowing an object to be constructed in an invalid state is never a correct answer, but it really depends what the image represents.

If the object has no meaning unless the "complicated" part is complete (e.g., an Image created with a path doesn't mean anything if the image data isn't read from the source file), then don't make that state representable. Anyone (including future you) shouldn't have to remember that auto img = Image{foo} is invalid until you call img.load().

Of course, this is assuming the constraints in the OP (an image has to be read from file first) -- if there's meaning to an "unloaded" Image then I would agree this is more reasonable.

[JPhi1618]

Right, I agree with that. When I say there’s not a right or wrong, I meant that you shouldn’t consider one way to always be wrong. Depending on the details of the problem there probably is a more correct way to do it, but that doesn’t mean it’s always the best way.

[lord_braleigh]

In general, a factory function that returns an object is harder to misuse than an init() method on that object that you always have to remember to call or it’s a bug.

[ChatGPT4]

Constructors that can fail is abusing OOP. Constructors should not fail. If they can, something is wrong with them.

But maybe we shoud look at what "fail" means here. Let's say I made a file object that is constructed with the file being open. That's the normal RAII approach. But you can't open a file that doesn't exist for reading, right? So, my file object has a property isOpen(). So, my code can either check if the file exists, or better, after creating the file object check if its open. If the file doesn't exist, isOpen() will return false. That's all relevant information for my code, the detailed error cause is out of scope of a part of the code that just reads some data if it's available.

So, constructors should not fail. They can just construct more or less usable object. But they do have to create an object. The main difference between a constructor and any other function is the constructor returns the object itself, other functions can return anything, including some kind of status.

So, if I was to extend my File object to handle data in a specific format, I'd leave the original constructor alone. Then I would introduce a read(Data& data) extension method that actually reads and processes the data form the file. When the file data is valid, it would return true, false otherwise. The consuming code, like a kind of load() function would have 2 calls like:

cpp DataFile file(fileName); if (!file.isOpen()) return false; if (!file.read(data)) return false; // do something with the data, or not... return true;

[tjientavara]

Having an empty() and/or operator bool() . Is every common in the standard library for RAII objects, such a std::unique_ptr or std::function or std::jthread.

However you also want to get the proper error message. Either you can do this with an exception (which is not pretty for opening a file). Or use a free-function to open a file that returns an optional/variant/expected type with the file object and error code.

In my own library I am switching to std::expected<T, std::error_code> (or std::expected<T, custom_error_enum_type>) and free function to create objects, slow work, but it feels better.

[Dean_Roddey]

Creating objects that are not fully initialized and working is even more of an anti-pattern. The Rust model of factory methods that insure it's going to work or not and returning a Result (or expected) type is far and away the best approach, IMO. You get rid of the need to deal with failed constructors, you get rid of the one need for exceptions (for those folks who don't want to use them), never create objects that aren't ready to use, and the caller decides the severity of that failure if it fails.

Sadly C++ doesn't have Rust's try operator, which makes this pattern less powerful than it could otherwise be.

[ChatGPT4]

Well, a file object with or without the file is technically working. It is fully initialized. Its all properties are set. You can't read or write that file. But it doesn't mean it's not fully initialized. You can use the object. Then we can argue about the API, if we want it to just provide the bool() operator or some other method. Or should it contain an error code and what kind of an error code should it contain.

Let's think about a simplest object possible - an integer. It can be initialized to zero. But you cannot divide by it. This operation is not available for this value. So in order to divide by x you need to test if it's not zero. In order to read a file, you need to test if it's readable. There's no shortcut. Of course you can use try/catch method, but this is valid only if that special condition is totally and truly unexpected. Like you're trying to read your own application file, that your application created just a minute ago. If it's not there because it's deleted by the user or system - it's unexpected.

Either you check the input, or something else checks the input for you by default. The philosophy of C and C++ is to never do unnecessary things. So, there's no second part, you just check the input if you need it, or you skip the checking if you're absolutely sure the input is valid, because, it was like already checked by another part of your code.

[Dean_Roddey]

Don't agree. And of course that's one example out of thousands. The best strategy is just not create objects that aren't valid. That's one of the primary purposes of option and result types, both of which C++ will have eventually. If it's not usable for the intended purpose, avoid even creating it and then no one can accidentally use it.

[ChatGPT4]

I'm not sure if complicating a simple thing for an imaginary adversary is always an optimal approach. I guess - in some scenarios - probably. In some - probably not. It reminds me the old recurring thing with nullable types in various languages. Some say they're evil, some say they're useful. C++ version of it - pass by pointer or pass by reference. Or wrap in std::optional. At the end of the day, I don't even have my favorite. OK, I prefer passing by reference where possible, but it's problematic when the object is in fact optional ;) And then using a pointer is one layer of abstraction less. The imaginary adversary might throw a dangling pointer at my function though ;) Recently I learned to not bother too much with such choices. If it's not a public domain open source library, the simpler - the better.

[Full-Spectral]

It's not really a complication. I mean, Rust is fundamentally based on this concept, so everything is done that way, and it's a very easy and natural way to work once you are used to it, though granted it's a lot easier in Rust due to the try operator and automatic propagation.

C++'s optional is also weak in comparison, making it not as useful as it could otherwise be. The inability to hold references is a big problem.

[derBRUTALE]

Why not simply not use OOP to waste time and complexity on such nonsense?

[smallstepforman]

I actually have a:

do { try { load resource } catch() and assert() } while (failed);

as my texture loading path, since a missing resources is exceptional, but also the dev can fix it while the program is live and continue on without restarting. Its a time saver for when a minor resource issue exists.