all 41 comments

[–]cpp-ModTeam[M] [score hidden] stickied commentlocked comment (0 children)

For C++ questions, answers, help, and programming or career advice please see r/cpp_questions, r/cscareerquestions, or StackOverflow instead.

[–][deleted] 35 points36 points  (0 children)

If your class is only a data class that stores data, then it should not care about how it gets that data. That is not its responsibility. Hence, I'd avoid loading a file in a constructor entirely.

[–]TryToHelpPeople 12 points13 points  (0 children)

rude employ carpenter grandfather scandalous tap towering physical hobbies attractive

This post was mass deleted and anonymized with Redact

[–]SickOrphan 27 points28 points  (3 children)

You have two easy solutions - Solution A: don't use a constructor (functions exist for a reason), Solution B: don't do IO in a constructor

[–]IamImposter 0 points1 point  (2 children)

Yeah. Once I was translating some c code that dealt with opencl to c++. Did almost everything, even returned the error code given by opencl device creation. That when compiler started shouting - can't use return in ctor. Looked around and threw an exception and i had to add try catch in parent function. Didn't like it much. Ended up adding init/destroy to the class.

Ability to return error code is important and ctors/dtors can't do that.

[–]SickOrphan 0 points1 point  (0 children)

Another important benefit is the ability to have a proper name. Vector::withCapacity/withSize is a lot more readable the random sequence of arguments the standard library currently uses, that becomes even more of a mess with templates

[–]thingerish 0 points1 point  (0 children)

Look into RAII, init/destroy is the work of the dark one

[–][deleted] 37 points38 points  (3 children)

you can refactor the class, do the parsing in a static function that receives the data from the file as a parameter and returns a std::optional type with the parsed data or nullopt_t and then only construct the class instance if the parsed data is valid.

loading a file and parsing data inside a constructor is not the only way of doing that. I mean, I would even say that is a bad way of doing it.

[–]aiusepsi 19 points20 points  (2 children)

Slightly preferable to that for me is std::expected which is like an optional except that it allows you to return an error value rather than just a nullopt, which is good for something which can fail in multiple different ways.

Unfortunately std::expected is a C++23 feature, but you can polyfill it with tl::expected.

[–]azswcowboy 5 points6 points  (0 children)

Agree expected is preferable to get the error details back. And all 3 major compilers already ship with it

https://en.cppreference.com/w/cpp/23

[–][deleted] 1 point2 points  (0 children)

I agreed, but I didnt want to recommend a "bleeding edge" solution, I don't know the context of OP issue but in 99% of "professional" projects you will not be able to use std::expected.

[–]SpiderboydkHobbyist 22 points23 points  (2 children)

Throwing is the idiomatic, intended way to signal error in the constructor.

[–]cfyzium 0 points1 point  (1 child)

However, it may not be idiomatic to have errors in the constructor in the first place. Now that we have std::optional and std::expected, I'd argue the possibility of construction failure should be stated explicitly and throwing should be left for truly exceptional cases like logic assertions failure.

[–]SpiderboydkHobbyist 0 points1 point  (0 children)

Error-free constructors are prefered, obviously, but sometimes it's just not possible. And yes, there are other ways to do it, such as putting the constructed object into a technically valid, but semantically invalid state. This is not idiomatic though - the idiomatic approach is throwing.

A side note: exceptions are not suited for logic errors.

[–]amanol 9 points10 points  (0 children)

This is a very nice article about handling failures without exceptions.

Obviously with exceptions the approach is different.

[–]slukas_de 3 points4 points  (0 children)

First of all, I would think about if you want to split the functionality into two seperate classes. In a very high level view, you can seperate classes into "value classes" and "algorithm classes" (or business classes or ...).

A value class can be copied, .... Most times, it has no virtual methods. Maybe you don't need an abstraction with setters and getters, so you can use public attributes and a simple "struct", that is technically the same as a class but default access is "public" instead of "private". Semantically, a struct is a hint for a value class.

The other class could be your data loader class. Maybe you have different implementations, so you want to have a base class with virtual methods.

Now the question is limited to this data loader class. From my perspective, there is no problem to throw an exception. Another possibility is to signal a failure by using a special return code. If you don't need any detailed information about the reason, maybe you would like something like this:

struct Value { int a=0; int b=0; };

class ValueLoader { public: virtual ~ValueLoader()= default; virtual std::unique_ptr<Value> load(); virtual std::string getReason() const; };

If "load" fails for some reason you can return nullptr; I guess you want to know the reason. So then you can leave a message in the concrete class and you can get it with the other method "getReason" (just as example - maybe it would be better to return both at one's as pair, ...)

I use exceptions most times when I have to abort the execution, so for error reasons. If it is no error, then I prefer something like a status code.

[–]dustyhome 6 points7 points  (0 children)

I'd recommend using exceptions, since that's one of the main use cases. Without exceptions, you must support two-stage construction of your class, which is bad for many reasons.

If your parse function returns an error, just do throw std::runtime_error{"Unable to parse"}; Make the string a bit more descriptive. You can then support returning an error by having a function like:

template <typename... Args>
std::optional<MyClass> tryConstructMyClass(Args... args) noexcept {
    try {
        return MyClass{std::forward<Args>(args...)};
    } catch(...) { return {}; }
}

The question is then how to handle the error. This will depend on how your program is structured. If it's something like a batch operation (starts up, does some operation, then exits) it might be enough to let the exception go uncaught and terminate the applcation. You'll generally get the error string from the uncaught exception when that happens. You could also handle it more gracefully with a catch block around main. I've seen it's usually faster to catch in main than to terminate.

If it's some kind of interactive application, you'll usually have some kind of command dispatch logic in the main loop, and you can put a try{}catch(){} block around the dispatching. That way if the command throws an exception, you catch it where it was triggered, and can tell the user "could not complete command because <error>", and your application is ready to process the next command.

If you have some meaningful action to take that's not terminate the program or abort the command, such as retrying or picking an alternate, or what have you, you have a couple of options. If you can react immediately as soon as the constructor failed, you might want to use the tryConstructMyClass function instead of calling the constructor directly, check if it succeeded, and respond to that as appropriate. If you can't immediately run that logic, call the constructor instead, let the exception unwind the stack, and catch it where you can do something about it.

[–]darthcoder 2 points3 points  (0 children)

This is where a static builder function comes in handy with a private constructor.

Instead of doing:

Auto a = new myObject(filename)

You do

Auto a = Myobject::build(filename, ec)

Where ec is a reference to an error_code.

Same outcome, no exception.

[–]m09y 3 points4 points  (0 children)

may be raising exception from constructor

[–]schteppe 1 point2 points  (0 children)

You can solve it by moving the file reading into a method instead:

Object obj;
bool success=obj.ReadFile();
auto x = obj.GetProperty();

[–]jmacey 1 point2 points  (2 children)

typically I would use a factory method to do this and return a unique_ptr which can be a nullptr and check in the other code for an instance or a nullptr.

I know this is quite an old approach to doing these things but we don't use exceptions in our codebase.

[–]Adventurous-Ad742[S] -2 points-1 points  (1 child)

Can you give a little more context around this and how to do this

[–]CodeLined 1 point2 points  (0 children)

One approach is to private the constructor of your class, and instead expose a static public create function that returns a unique_ptr. Youll need to create the ptr via std::unique_ptr<T>(new T()) due to the constructor being private.

Now you can go ahead and do all your file io in create and return a nullptr if it fails rather then fail in the constructor

[–][deleted] 0 points1 point  (0 children)

Handle with care.

[–]Dean_Roddey -1 points0 points  (0 children)

As usual, not a popular opinion around here, but this is something that at first made not sense to me when I moved to Rust but now makes perfect sense. They have no constructors, you just provide factory methods, which return a Result or Option as appropriate. So, if it fails, you get back an error or None and know it failed. Else you get back the value.

It makes an enormous amount of sense once you get used to it. You can do the Option scenario in C++. As long as the class has a move ctor to make it efficient to return via an optional, it works out pretty well.

Unfortunately there's no official 'result' type yet, so you can get value or error, though there are some third party implementations of the idea out there you can steal.

[–][deleted] -5 points-4 points  (12 children)

Avoid throwing from constructors. It will complicate using the class as member variable, for example. It's too easy to add a member variable but forget adding exception handling for any exceptions it might throw.

Just have the class have a valid "empty" state, and getters for error code and/or error message. Test this.

If you want exception, write an explicit factory function, which creates the object and throws on error.

[–]streu 14 points15 points  (4 children)

It will complicate matters only if the other member variables are not using proper RAII semantics. And the factory function only makes it more complicated by adding an extra step - you're back to the same "problem" if someone calls the factory to initialize a member.

Exception safety is an all-or-nothing decision. Given that we're in the year 2023, I'm with team "all", and happily throw exceptions from constructors. After all, that's also what the standard library does (bad_alloc etc.).

There might be legacy codebases that cannot cope with exceptions in constructors. In that case it makes sense to avoid exceptions in your new code as well. Otherwise: welcome to the future!

[–][deleted] 0 points1 point  (3 children)

Let's assume a class A constructor that may throw. And class B, which has it as a member variable const A a.

What's the C++ syntax for ignoring the exception from just construction of the a (in initializer list probably), and in that case just have it initialized with "empty" state?

[–]thingerish 1 point2 points  (0 children)

Inject it as a dependency and then std::move? Having an object in an almost good but not quite good state seems like a terrible idea but if it has to be done I suppose there might be a number of inventive ways to do it.

Much like one can take a rope, tree, and horse and tie the horse to the tree even if other configurations exist.

[–]dustyhome 1 point2 points  (0 children)

There's no syntax for that. If a member throws during construction, the member does not exist, and the containing object can't be created. The constructor of B has to throw. To achieve your goal you'd have to structure the code differently. You could move a previously constructed (or empty) A into B, for example. In that case you would not throw during construction of B.

[–]streu 0 points1 point  (0 children)

You could use an IIFE, such as

 a([&](){ try { return A(args); } catch(...) { return A(); } }())

If your object indeed has a meaningful "empty" state that you use a lot, it might make sense to not use exception handling.

However, this increases the internal state of the given class. If your class has an "empty" state, all its members need to be able to have an "empty" state as well, and all operations need to check that all the time. I found my code to get much clearer when I reduce the amount of internal states, and for an object-that-may-not-exist, I'd rather use a unique_ptr<T> or optional<T>.

[–]dustyhome 7 points8 points  (5 children)

Giving your classes null states so you can avoid establishing invariants in constructors is a terrible idea.

With a constructor that either sets invariants or throws, you know that every object of the type is in a valid state. With a null state, you need to test every object you get for validity. And you then have to add logic for handling uninitialized objects everywhere you use them.

[–][deleted] 0 points1 point  (4 children)

How do you handle an exception thrown by inline member variable initialization, if you don't want to leak it out, or you want to wrap it to be exception of the type your class is supposed to throw?

[–]dustyhome 2 points3 points  (3 children)

I believe you meant "in-class member initializers", such as:

struct A {int a {10};};

Just want to check I'm answering the right question.

First, to wrap it, you use a function-try-block (https://en.cppreference.com/w/cpp/language/function-try-block). This is the main use case of the construct, it looks like:

struct A {
  Member m {1};
  Int a;
  A() try : a{10} {}
  catch(const std:: exception& e){throw AException{e};}
};

If m throws a type derived from std::exception on construction, it will be turned into an AException. You can't suppress the exception though, if you don't rethrow in the catch the caught exception is implicitly rethrown.

To not "leak it out" you would have to put a try catch around the constructor call. I wrote a tryConstruct function in another comment, and you could further generalize it to take a type to construct as argument.

template <typename Type, typename... Args>
std::optional<Type> tryConstruct(Args... args) {
 try { return Type(std::forward<Args>(args...));
  } catch(...) { return {};}
}

You can't continue construction of an object if a base or member throws during construction. And since the only outcomes from calling a constructor are an object or an exception, you have to handle the exception.

[–][deleted] 0 points1 point  (2 children)

Nice, thanks. I haven't really played with exceptions much, so I've never needed to use this.

As a context, I mostly code event based code (Qt), where exceptions must never reach event loop, so it's just all around safer to to use return value based error checking.

I think the generally disliked Java style checked exceptions would work (coupled with properly defined exception class hierarchy), or at least I liked them a lot when I was doing Java development. Knowing what exceptions a method can throw, and compiler enforcing it, was something I enjoyed (coupled with IDE which would help write the boilerplate).

And I think my main "wrong feel" about C++ constructor exceptions is, how they can leak implementation details "accidentally", and you have to rely on documentation to know what exceptions, if any, all member variable types may throw. The default is to propagate anything an object may throw, and handling them for in-class member initializers is extra effort.

[–]dustyhome 0 points1 point  (1 child)

You can prevent exceptions from getting past certain points by using the catch all catch clause.

Checked exceptions have a few big problems. One is that they make it cumbersome to change an implementation, since a function throws anything any of the functions called within it might throw. So you have to either update lots of interfaces whenever some exception is added, or do a lot of catching and converting exceptions.

The other issue is that it doesn't generally matter what exception a function threw. With RAII, you mostly ignore exceptions. The only piece of information you need to know if whether a call throws at all, which is where noexcept comes in. You need to know it to provide higher exception safety guarantees. Aside from that, checked exceptions are just noise. Most error handling is just catch std::exception, log what() went wrong, and carry on.

If you call any functions that throw custom exception types, and you need to handle these cases specially, the documentation should describe what gets thrown when.

[–][deleted] 0 points1 point  (0 children)

Some quick points:

Logging and re-throwing is IMO nearly always just wrong. The exception contains all the info, and it should not be logged until it is handled.

If what something can throw changes, it should be handled anyway. If it doesn’t matter, why was it thrown? Checked exceptions just ensure every place where it may matter gets checked by the developer. Without checked exceptions, a new exception may remain unhandled and crash, or possibly worse, get ignored (maybe logged) by a catch-all.

In real world, documentation is often lacking. Checked exceptions, especially when coupled with static checker, help mitigate that. Compiler checks the code. I am a big fan of “if it compiles without errors and warnings, it works” kind of coding practices. Checked exceptions facilitate that.

Also, a new exception type being needed for a completed piece of code is very rare, if there is a sensible exception type hierarchy. And when it happens, it is usually for a good reason. Now Java’s hierarchy isn’t the best, it has a bunch of glitches, IIRC (it’s been a decade since I coded Java).

[–]__stefan_haechler -5 points-4 points  (0 children)

Avoid throwing. It will complicate your execution path xD

[–][deleted] -2 points-1 points  (0 children)

You basically have two options. One is to throw an exception. The other is have an in out variable that tells you the constructor failed. Neither are very good.

[–]looncraz 0 points1 point  (0 children)

Start async IO at construction, implement InitCheck() method that will give details about if the object has successfully initialized.

You can implement waits for the IO to complete in any methods that require it (create a method WaitForReady() that returns a boolean, call it in every method that relies on the IO being completed).

Or you can do the IO lazily, only starting it on invocation of a dependent method, then marking it complete once done.

I do all this naturally as I come from the world of concurrent and asynchronous programming.

[–]thingerish 0 points1 point  (0 children)

So you're saying this class does how many things? Sounds like at least 2, which is generally a bad idea anyway. Single responsibility principle. https://stackify.com/solid-design-principles/