Thread: To throw or not to throw

Well, I mean, they're different calls. So:

DoIo();
DoIo2();
DoIo3();
DoIo4();

Not really trivially turned into a loop.

We're not going to allow that to propagate.

O_o

Why?

You could try to refactor this into a function, and you would get:

You could split the translation code into a function, or you could combine both mechanisms into a private interface.

Code:

OurMyIoOp(); // We allow a private implementation
OurMyIoOp(); // to transform the exception on
OurMyIoOp(); // our behalf instead of duplicating code.

The difference is now that the error handling has been moved to the catch handler instead of having to write verbose code for every line to check for errors.

This comment is wrong.

You aren't handling the error; you are only translating the error.

The difference is that the translation code has been moved to a local catch handler so that the check doesn't need to be duplicated, wrapped, or similar.

Scenario 3. The library throws returns an error.

o_O

Which?

We can use a macro or function with a lambda to deal with that, which makes it a little bit better, but still verbose:

You could split the swallowing code into a function, or you could combine both mechanisms into a private interface.

Code:

OurMyIoOp(); // We internally swallow the exception.

Sometimes we can use helper functions such as CanOpenFile which [...] attempts to open the file.

We are talking about a situation where we have one or more known alternative to accomplish a similar goal in the case of failure; we are talking about a situation where the code can not continue when one of the alternative succeeds. You can almost always use create a `OpenOrCreateFile' function. The combined interface internally handles the attempts to allow each alternative to succeed.

Code:

auto_ptr<MyFile> s(CreateOrOpenFile(/* ... */));

We may then handle errors from interfaces that have no alternative using some other discussed method.

Code:

try
{
    auto_ptr<MyFile> s(CreateOrOpenFile(/* ... */));
    // We may wrap the translation code if we get a return value.
    OurReadFile(s, /* ... */); // if(ReadFile(s, /* ... */) != OK) // throw Whatever(/* ... */);
    OurReadFile(s, /* ... */); // if(ReadFile(s, /* ... */) != OK) // throw Whatever(/* ... */);
    OurReadFile(s, /* ... */); // if(ReadFile(s, /* ... */) != OK) // throw Whatever(/* ... */);
}
catch(/* ... */)
{
    throw Something(/* ... */); // We may translate an exception.
}

The diference here is that we potentially have many try/catch statements and not just a single global one that covers all exceptions.

We our C++ programmers. We do not accept a repeating "potentially have many try/catch statements and not just a single global one that covers all exceptions" problem throughout our client code. We translate the errors we need behind a private interface so that we do not have to repeat ourselves.

Code:

auto_ptr<MyFile> sIn(CreateOrOpenFile(/* ... */)); // Try the alternative options allowing other 
auto_ptr<MyFile> sOut(CreateOrOpenFile(/* ... */)); // failures to propagate or translate the errors we need.
try
{
    OurReadFile(/* ... */);
    OurWriteFile(/* ... */);
}
catch(/* Read */)
{
    // Whatever
}
catch(/* Write */)
{
    // Whatever
}
catch(/* ... */)
{
    // Whatever
}

[Edit]

By enabling the library to conditionally throw an exception.
It doesn't change the fact that we have to handle the error. It changes where we handle the error by making the code less verbose.

Indeed. Everything I described about handling the different approaches to reporting an error may be handled internally so that the relevant boilerplate propagation/translation code isn't necessary.
[/Edit]

Soma

Originally Posted by Elysia

How do we do this? By enabling the library to conditionally throw an exception.

This approch is a recipe for disaster. You end up having to write the code for both approches, which defeats the whole point of exceptions, that being to save you from the tedious hassle of error propegation.


The best approch is to be up front with expected behavior. Consider the configuration senario:

Code:

config_t loadConfigFile( string fn )
{
    ...
}

config_t loadConfigFileOrDefaults( string fn )
{
    try {
        return loadConfig(fn); }
    catch(Exception&) {
        return getDefaultConfig(); }
}

It's all intuitive. loadConfigFile() will do just that, or it will throw. If you really don't want to deal with that failure, use loadConfigFileOrDefaults() instead, which, again, does exactly what it says on the tin.

Originally Posted by Yarin

This approch is a recipe for disaster. You end up having to write the code for both approches, which defeats the whole point of exceptions, that being to save you from the tedious hassle of error propegation.

I think you are misunderstanding. When I say "conditionally," it means that for every function, we explicitly tell the library if we want it to throw or not. That way we ARE upfront about whether it throws or not and you can CHOOSE if you want it to throw or not. We don't want to write helper functions for every piece of library code if possible to do it easier. That's just unnecessary boiler plate that needs to be added to every project that uses it.

This approch is a recipe for disaster. You end up having to write the code for both approches, which defeats the whole point of exceptions, that being to save you from the tedious hassle of error propegation.

O_o

Wrong! You would only have to code for both approaches if the library is incredibly bad.

If we are going to assume "incredibly bad", why are we even talking? Throw the crappy library out or wrap the components you need.

If we assume a competent design, we only need to code for the approach we choose to use as a client.

Let's take an example from the standard library. Actually, let's first pretend we are building a common "RAII" component.

We are going to change the state of an object derived from a `std::?stream' type. Our object has a lot of state that can be changed to benefit our particular needs, but we can't just arbitrarily change the state of an object we don't own so we need to revert the state as it was before we specified the options we need. We are going to create the class `IOSState' which exposes the underlying mutators and accessors for those state of the `std::?stream' object we need to use.

Are you done? (I'm am pretending that you did for the sake of discussion.) Good.

Now, we can look at our options for using the `std::?stream' object.

We can, using `std::ios::exceptions', request certain operations to fail by raising an exception instead of returning an error code.
We can, using `std::ios::exceptions', request certain operations to fail by returning a return code instead of throwing an exception.

For the example usage, we can't continue if `badbit' is set. However, we expect that we may be reading the last bits of the stream so `eofbit' is not an error.

Code:

void DoSomething
(
    std::istream & f
)
{
    int sValue;
    bool c(true);
    do {
        try
        {
            f >> sValue;
            // We can use `sValue' as we need.
        }
        catch(...)
        {
            if(f.bad())
            {
                throw Whatever(/* ... */);
            }
            else
            {
                c = false;
            }
        }
    } while(c);
    // We can do more stuff.
}

We don't have to `catch' only to inspect a value to decide about using `throw' to raise some other exception.

Code:

void DoSomething
(
    std::istream & f
)
{
    int sValue;
    int sCode;
    while((sCode = (f >> sValue)) != EOF)
    {
        if(sCode == GOOD)
        {
            f >> sValue;
            // We can use `sValue' as we need.
        }
        else
        {
            throw Whatever(/* ... */); 
        }
    }
    // We can do more stuff.
}

We also don't have to inspect an error code to decide if we have good value or if we have an error we can't handle.

By allowing us to specify which errors we can handle and which ones we can't, the library confirms to our requirements; we can choose to handle `EOF' as an expected event while allowing errors we can't handle to propagate as exceptions.

Code:

void DoSomething
(
    std::istream & f
)
{
    IOSState sRollback(f);
    sRollback.exceptions(badbit);
    int sValue;
    while(f >> sValue) // We allow the exception to 
    {
        // We can use `sValue' as we need.
    }
    // We can do more stuff.
}

The best approch is to be up front with expected behavior.

Look though at your scenario. Your solution to the scenario is designed to swallow problems so that the client never knows about any errors. From the perspective of the client, you always provide a valid object. You can't always swallow problems. You will sometimes need to provide information to the client about any problems. The choice between error codes, exceptions, or allowing the client to choose either of the others is about informing the client about a problem you can't swallow.

You are returning an object which, I believe, is potentially created by parsing a file or a default form of that object so that client code is guaranteed to get a useful object. I agree that, in the given scenario, your approach is sublime. I would accept nothing less; I would either get a different library or wrap the underlying components. We can handle the problem, for that scenario, so we shouldn't force the client to care. When guaranteeing a client success is an option, I completely agree with your argument.

Soma

Originally Posted by Yarin

I understood that. What I'm saying, is when you do that, you have to write code inside said functions to perform that. Said extra code is going to be mostly the same code that's used in the traditional C-style error handling. At this point, why even use exceptions, if the library is littered with non-exception error propegation code in the first place? The "convenience" this gets you comes at the cost of bloat and repetitive code.

Code:

void DoIo(...) // Throw
{
    error err;
    DoIo(..., err);
    if (err != OK) throw Exception(err);
}

void DoIo(..., error& err_out) // Does not throw
{
//...
}

Is this a lot of bloat and repetitive code? You're not going to get away from that, btw, since you need to code somewhere, in some form.

Besides, there are basically three ways of doing this:
You do it in the library, keeping the library a little more verbose and the client clean.
You do it in the client with no helper function, making each library call verbose.
You do it in the client by creating a helper function, making the helper function verbose, but the rest of the client clean. Of course, you have to copy those helper functions to your next project that uses the library.

"helper function" is a misnomer. All functions are helper functions, otherwise they shouldn't be in the code base.

I disagree with that. A helper function is a function that does not directly contribute to the application logic. It is a function that is called by such functions in order to perform some tasks in (possibly) many different places in an application. I believe some may call them utility functions. These are clearly separated from functions that do directly contribute to logic, such as Init, RunUI, etc.

So a helper function is like the first DoIo. It translates a return code into an exception. It doesn't do things like update UI, initialize app, etc. Libraries are typically full of utility/helper functions.

Originally Posted by Yarin

That first function shouldn't need to exist,

Incorrect.

Originally Posted by Yarin

the second one should be throwing on it's own.

Incorrect.

Originally Posted by Yarin

In order for that second function to pass the errors back out to the caller, it has to be tediously handling error propegation itself. Again, why even have exceptions, if you're not using them where they're most helpful?

Because sometimes I don't want an exception to propagate! See my scenarios. Sometimes it's helpful. Sometimes it's not.

This is a straw man argument, because it rests on the premise that an EOF condition is an error.

O_o

Nope. I specifically said "However, we expect that we may be reading the last bits of the stream so `eofbit' is not an error.".

You even quoted that part of my post.

Would you like to try again?

In my programs, EOF is only ever treated as an error if the consumer cannot complete it's task upon encountering it, in which case, it certainly is an error and deserves to be thrown.

Congratulations! You are correct.

The `EOF' state can indeed be a normal occurrence.

The `EOF' state can also certainly be an error condition.

It's just hypothetical code, i didn't think to handle the error.

You did a fantastic job breaking the code!

You've forced the client to report an error (The `err' in `std::cerr' stands for "error".) even if the situation is actually normal from the client's perspective.

The client can't now treat the configuration file missing as a normal occurrence.

[Edit]
I actually just kind of assumed you were okay swallowing the exception to guarantee an object in the face of problems loading the configuration.

If you are serious about "handling" exceptions internally with a log trace, what are you going to do if `getDefaultConfig' raises an exception?
[/Edit]

Soma

Of course, you know, the streams do it neater because their flags are tucked away in objects.

O_o

The code Elysia posted is actually better than the form you posted when you only care about the single question.

Your version forces the client to use an extra parameter even if the client would rather see an exception raised to signal an error.

Code:

error err_out;
DoIt(..., err_out, true);

You could eliminate the requirement using a pointer instead of a reference.

Code:

DoIt(..., 0, true);

You could invert the default and also default the pointer value.

Code:

error err_out;
DoIt(...); // throws
DoIt(..., &err_out, false); // nope

Using the overloaded convenience function with an operational reference is simpler.

Code:

error err_out;
DoIt(...); // throws
DoIt(..., err_out); // nope

The use of the optional reference argument informs the implementation to raise an exception or return an error code.

Soma