Thread: RFC on an approach to address the "return boolean or throw exception" quandary

Originally Posted by kmdv

So don't define it as an enum with predefined "bad" code (multiple names for the same thing just shocked me...).

I'm not sure I follow you there. Do you mean only define a successful state? Or what?

The multiple defines are nothing really unusual; some prefer the "good"/"bad" naming convention, others "success"/"failure" (or simply "fail"). I was just accomodating the most common choices.

Originally Posted by kmdv

I consider it a bad usage, not a taste.

How so? Because it doesn't "feel right"? Okay, so rather than shame those who prefer the functor syntax, suggest an additional named function. Being that the error class only has "setters" for functions that take arguments, it's perfectly logical to opt for a functor syntax; it's clean, simple, and unambiguous, I think.

Originally Posted by kmdv

It's not about technical problems (which could be removed with new usage of the 'explicit' keyword) it is just confusing. For example, implicit conversion to bool for shared pointers is obvious, since C++ allows such for raw pointers.

What's so confusing about an "if(error)" idiom? Or equating an error with a code? Assuming the user has RTFM before using the class, there shouldn't be any confusion anyway. And again, a named function for those who prefer it is reasonable as well, so both the "explicit intent" and "syntactic sugar" camps could be accommodated, anyway.

Originally Posted by kmdv

It's undefiend behaviour, there is no discussion. And I certainly wouldn't call it a practical way.

Agreed, it wasn't the best solution, but I already suggested a reasonable alternative (ie: the "shared handle" route). And by "practical" I simply meant that there weren't any other obvious techniques at my disposal. I certainly welcome any suggestions, of course.

Originally Posted by kmdv

Because you are not going to catch every single kind of exception. Base class should have a virtual getter to allow derieved classes to reimplement their own ones.

Okay, good point. Maybe it'd be a good idea to derive the class from std::exception, too?

Originally Posted by kmdv

Just because it is defined and it compiles does not mean it is good.

Admittedly, I hadn't really thought enough about it to grasp the true implications there. A few simple test cases were enough to convince me, though, that the current usage is unacceptably unsafe. Namely, instatiating an error object directly is what can wreak so much havoc. Unfortunately, there's really no way to make the constructors private/protected without rendering the class unusable, so the only viable option that I can see would be to demand that the user either check or record the return value at the call site or else catch the exception. Direct instantiation would be forbidden and the consequence undefined behaviour.

Originally Posted by kmdv

By that I meant that this usage is very unintuitive.

Can't please everyone, I suppose...

Originally Posted by brewbuck

Other cool tricks you can do with this...

Templatize it so it wraps a generic error code (int, enum, whatever), instead of limiting to just bool.

Give the ability to discard an error code deliberately without checking it. You can ignore an error, but you need to call a function to explicitly do it.

Give the ability to detect positive vs. negative status, for instance you can trigger a breakpoint whenever an error is first generated, but continue along quietly when things are going fine.

Make the whole thing controlled by a compile-time flag. When disabled, the return codes all revert back to plain old int/bool/enum again. Now your debug builds can have really sweet error tracing, and your release builds don't suffer from it.

Remember that this is all great for debugging, but not at all suitable for an error code architecture.

Great suggestions! Could you please elaborate on the third comment, though? I'm not exactly sure what you meant by that.

Originally Posted by gardhr

Great suggestions! Could you please elaborate on the third comment, though? I'm not exactly sure what you meant by that.

Suppose you're writing a library with some API called UsefulFunction():

Code:

int UsefulFunction() // returns 0 on success, negative on fail
{
    DoSomeStuff();
    int status = SomeInternalFunction();
    if (status < 0)
        return status;
    DoSomeOtherStuff();
    return 0;
}

A user of the library calls UsefulFunction() and it returns -1 (something bad happened). Now it is your task to debug this failure. Well, the failure must have come from SomeInternalFunction(). But why did THAT fail? Maybe SomeInternalFunction() calls functions X(), Y(), and Z(), all of which can fail, and you don't know which. Maybe it was Z() that failed. But Z() calls A() and B()... Etc.

This is a headache which usually requires a lot of repetitive stepping to figure out exactly what FIRST went wrong to trigger the cascade of error codes.

Now imagine you are using a status class like the one you've written here. You could put some code into the constructor and assignment operator which checks to see if the raw error code is negative -- if it is, you can trigger a breakpoint.

With that in place, you can reproduce the failure case, and the code will immediately break at the lowest layer where the error was actually generated in the first place. No detective work required.

Originally Posted by gardhr

I'm not sure I follow you there. Do you mean only define a successful state? Or what?

The multiple defines are nothing really unusual; some prefer the "good"/"bad" naming convention, others "success"/"failure" (or simply "fail"). I was just accomodating the most common choices.

When you look at code it is not obvious whether the type of success equals the type of good. You are also not sure whether these two have the same integral value.

Originally Posted by gardhr

How so? Because it doesn't "feel right"? Okay, so rather than shame those who prefer the functor syntax, suggest an additional named function. Being that the error class only has "setters" for functions that take arguments, it's perfectly logical to opt for a functor syntax; it's clean, simple, and unambiguous, I think.

There are many things that you prefer now but which you will find later a total mess. Yes, it doesn't feel right. One or two setters would be more appropriate, but I would even consider removing this feature.

Originally Posted by gardhr

What's so confusing about an "if(error)" idiom? Or equating an error with a code? Assuming the user has RTFM before using the class, there shouldn't be any confusion anyway. And again, a named function for those who prefer it is reasonable as well, so both the "explicit intent" and "syntactic sugar" camps could be accommodated, anyway.

I wasn't precise about that (it was related to my previous post where I talked about the conversion to int) and the bool example misleaded you. Conversion to bool is fine, I meant the conversion to int.

Originally Posted by gardhr

Agreed, it wasn't the best solution, but I already suggested a reasonable alternative (ie: the "shared handle" route). And by "practical" I simply meant that there weren't any other obvious techniques at my disposal. I certainly welcome any suggestions, of course.

The whole m_throws_ mechanism is suspicious to me. If you are going to modify the state of the object being copied, it is not a solution. Using the mutable keyword is also NOT the solution. m_throws_ is a part of object's state and such an object should be passed by a non-const reference in case you want to modify it.

Originally Posted by gardhr

Okay, good point. Maybe it'd be a good idea to derive the class from std::exception, too?

You could derieve from runtime_error, which implements the what() member function. It has its pros and cons. The main idea is to have one base for all (or at least as many as possible) application-specific exceptions.

I would like to add something about your naming convention, which obviously follows the one from STL - ugly underscores, abbreviations, confusing getters/setters (overloading), etc.
It is a matter of style of course, however, camelCase (or CamelCase, which I prefer) is seen much more often in projects, I guess.

I couldn't edit:

Originally Posted by kmdv

The whole m_throws_ mechanism is suspicious to me.

Yes, shared state would be a reasonable solution.