Return success value - Best Practice

[pkdir]

Hi,

I have a few functions that load and validate data from a file. If any of the text fails a given condition, I want my program to output an error and terminate.

Currently my functions print out the specific error message and return a bool to indicate success or failure.

Pseudo code:

bool validateFile(std::string path)
{
    // load/read file

    if(!conditionOne)
    {
        std::cerr << "Error Message ..." << std::endl;
        return false;
    }

    if(!conditionTwo)
    {
        std::cerr << "Error Message ..." << std::endl;
        return false;
    }

    return true;
}

I was thinking instead I could return some sort of 'Validation' struct that contains a bool for success and an error message if applicable. The caller can then check the status and output the message if required.

Is this over-complicating the issue or would it be considered better practice then just returning true/false?

[jlb]

If any of the text fails a given condition, I want my program to output an error and terminate.

If you always want the program to terminate because of an error, why not throw an exception?

[jonnin]

Its a better design to return an error message alone, just a string, IMHO. Then if the string is empty, it was successful, if not error out. Adding a bool to it seems redundant.

If you want you can chain errors, rather than return right away. So if the problem entry had 4 problems, you can append them

if(cond1)
stringvar += "error 1. ";
if(cond2)
stringvar += "error 2. ";
…
output
error 1. error 2. …

[pkdir]

I'm learning in a course, and while this question isn't for any specific work (just learning) we haven't touched on exceptions yet. I have used them in Python before, but every time I read about them in the context of C++, a lot of people seem to frown on them.

As a tangent if I were to rephrase my question and say I didn't want to terminate, just get the status and output the error message if it exists, which method would be considered best?

[jlb]

As a tangent if I were to rephrase my question and say I didn't want to terminate, just get the status and output the error message if it exists, which method would be considered best?

There is no one "true" answer to this question.

Also, IMO, this question is not very specific. For example what do you mean by "get"?

Also, IMO (again), I prefer to return an integer value for errors, zero means no error, anything else means there was an error of some kind (the value could be an indication of the actual error).

[jonnin]

My approach does not care what you do with the result. You can treat it as a Boolean (has error or not) or use it for output (this is what happened) or throw an exception or ignore it entirely.

its identical to JLB's
I prefer to return an integer value for errors, zero means no error, anything else means there was an error of some kind

except its a string instead of int. His approach, if he wants to print a message, he has to convert his int to the output; mine rolls that in at the cost of returning a fatter object.
you can split the diff and put a tiny bit of text in an int. 8 ascii letters and a bool will all fit in 1 64 bit int :P but that is kind of silly. 9 letters will fit if you go 7bit on it.

[pkdir]

well if I were to implement it:

struct Result
{
    bool success;
    std::string message = "";
};

Result validateFile(std::string path)
{
    // ... open/read file

    if(!condition)
        return { false, "Error Message..." }

    // ... Other conditions

    return { true };
}

int main()
{
    Result result = validateFile("file/path.txt");

    if(!result.success)
    {
        // Handle error, can retrieve specific error message if needed
    }
}

Thanks for your view though, this is all I'm after :)

[jonnin]

Its your code, and that is perfectly valid, but again, it is also redundant. You can do all that in a string without the struct, so you have a tiny inefficiency.

if(!result.success)
is virtually identical to
if(errortxt.length()) //renaming message makes this readable.

[pkdir]

I keep replying in-between you posting haha.

So after reading your message

My approach does not care what you do with the result. You can treat it as a Boolean (has error or not) or use it for output (this is what happened) or throw an exception or ignore it entirely.

I think I am liking this method.

I definitely prefer the readability of if(!result.success) but as you mentioned renaming the message variable seems to make it readable enough.