W
wij
Note: This thread is from comp.lang.c++.moderated, moved here
because I failed twice to reply. Please refer the original
thread for context.
I am just saying one thing that "throwing error is potentially buggy".
Not that it is convenient for an error string to be written to a log
file, nor that it is elegant to mean stack unwind to exit. I am not
even really talking about simplicity neither, even whether it is an
real error or not, but merely correctness of the error reporting
mechanism. Again, I am not talking about any implement of your or his
or her correct throwing strategy, but the prevailing, the frequently
taught, probably the 'standard' one.
Let's first confine errors to only errno-like classes, assume they
all inherit from std::exception.
class E_PERM;
class E_NOENT;
class E_SRCH;
class E_INTR;
class E_IO;
class E_BADF;
class E_AGAIN;
class E_NOMEM;
class E_NOSPC;
class E_INVAL;
class E_PIPE;
class E_TIMEDOUT;
class E_NAMETOOLONG;
class E_FBIG;
class E_LOOP;
.....
// Class String has all similar members as std::string
// except throwing E_... class
//
class String {
// Take one for instance:
// Throw: E_INVAL $index is invalid
// E_FBIG String length too long
// E_NOMEM Not enough memory
//
void insert(size_t index, const String& str);
};
Now we are writting a funcion process_msg. All called functions throw
E_... in exceptional condition.
// Throw: E_INVAL $in is invalid
// E_FBIG result string is too long
// E_NOMEM not enough memory
//
void process_msg(const String& in, String& out);
{
MutexLock lock(mutex); // Not likely to fail. But shouldn't
// a robust program
catch the possible
// E_INVAL, E_PERM,
E_AGAIN from leak?
String s1,s2;
if(in.size()<=0) {
throw E_INVAL;
}
db.read(s1,1000); // may throw E_INVAL, E_FBIG, E_NOMEM,...
parse(s1,s2); // may throw E_INVAL, E_FBIG, E_NOMEM,...
process(s1,s2,out); // may throw E_INVAL, E_FBIG, E_NOMEM,...
}'
Then, a tiny part of the loop in the server-like program:
try { process_msg(in,out); }
catch(E_INVAL& e) {
send("Invalid argument",e ); // peer end could have a wrong message
// Probably one want to log message here, but there are more story
// about stack unwinding that can occur here. If error is
// misinterpreted, the program goes to std::terminate for the good.
// The error report is not just for logging purpose.
// This is one reason a function should be somewhat responsible
// to its declared thrown objects.
}
catch(E_FBIG& e) {
send("Result too long",e); // peer end could have a wrong message
}
This pattern has been explained several times. It should be clear
that at least all process_msg(..) documented errors should be caught
(probably convert to another one) in the function process_msg(..).
The prevailing "catch only those you know how to handle" style and
the "rethrow error" preaches won't work well in such error handling
or message passing scenario.
So the revised version (not the popular style):
void process_msg(const String& in, String& out)
try {
MutexLock lock(mutex); // Note: E_INVAL, E_PERM, E_AGAIN might be
// thrown here, but no
way for a local
// catch. So we added a
try/catch layer.
String s1,s2; // E_NOMEM might be thrown
if(in.size()<=0) {
throw E_INVAL; // this is the responsible one
}
try { db.read(s1,1000); }
catch(E_INVAL&) {
// What to throw? process_msg(..) only documented 3 errors.
// Do we need more errors to report? Yes, so add E_IO to mean
// vague error.
throw E_IO;
}
catch(E_INTR&) {
// Let this be handled correctly
}
catch(E_IO&) {
// E_IO had been decided for a vague error. So this error
// can't be directly reported.
throw E_IO;
}
catch(E_PERM&) {
// process_msg(..) intends to hide the implement detail that it
// reads data from a database or config file. So throw E_IO.
throw E_IO;
}
catch(E_FBIG&) {
// To be defensive. Not knowing how 'db.read(..)' will actually be
throw E_IO;
};
// There are other possible thrown type by definition.
s1+=in; // Lucky, E_FBIG is appropriate
try { parse(s1,s2); }
catch(E_INVAL&) {
throw E_IO;
}
catch(E_NOENT&) {
throw E_IO;
}
catch(E_FBIG&) {
throw E_IO;
};
// There are other possible thrown type by definition.
try { process(s1,s2,out); }
catch(E_INVAL&) {
throw E_IO;
}
catch(E_RANGE&) {
throw E_IO;
}
catch(E_BADMSG&) {
throw E_IO;
}
catch(E_FBIG&) {
throw E_IO;
};
// There are other possible thrown type by definition.
// Can the above annoying catch handlers be saved?
// My answer is no.
}
catch(E_INVAL&) {
throw; // *** BUG ***
//E_INVAL may be from the lock failure. There
// are occasionally similar codes causing such
// conflicts.
};
See, it is not easy for process_msg(..) to be merely responsible for
those documented throw types. To add to this burden, this style
require all functions of the program to follow this coding standard.
Note: Some approaches use specific name space or throw classes, but
they basically only reduce the chance of being misinterpreted, with
lots of unbalanced costs. Still, not the final solution.
because I failed twice to reply. Please refer the original
thread for context.
True, it's not really appropriate to show the complete code. But it's
possible to show overall structure (see below).
OK. So show some detail. How do you plan to handle error return from
receive, process, and send? Do you proceed with process if e.g.
receive fails? (I guess not). Do you "send" if "process" fails? (I
guess not.) How are receive, process and send declared? What are their
failure modes? How do you propose to signal and handle failure of
receive, process and send? You speak about possible errors, but in the
loop you've shown, there's no visible error conditions, no error
logging, no recovery, nothing. You only came up with a vague
impression that there's something wrong, but shouldn't you show, in
small snippet, what is that? People here gave you same advice, but you
as far as I can see, completely disregarded it.
Just show a rough sketch of anything using error-return, and I'll make
you a similar sketch using exceptions. It will do __exact same thing__
(that is, it will be "correct"), and there's a very big chance that it
will be shorter.
But OK, I'll give it a shot. Here's your loop with some very basic
error reporting:
// Any of these return false in case of failure
// (I presume errno is set by lower level code, but that's rather
irrelevant).
bool receive(MSG&);
bool process(const MSG&, OUT&);
bool send(const OUT&);
void log_error(const char* what)
{ // some logging.
cerr << what << " failed, errno: " << errno << endl;
}
while (true)
{
MSG msg;
if (!receive(msg))
{
log_error("receive");
continue;
}
OUT out;
if (!process(msg, out))
{
log_error("process");
continue;
}
if (!send(out))
log_error("send");
}
Or, alternatively, you could try:
while (true)
{
MSG msg;
if (receive(msg))
{
OUT out;
if (process(msg, out))
{
if (!send(out))
log_error("send");
}
else
log_error("process");
}
else
log_error("receive");
}
Here's the same thing using exceptions:
// Any of these signal error by doing throw "what" (e.g. receive will
do throw "receive").
// (Again, I presume errno is set by lower level code, but that's
rather irrelevant).
void receive(MSG&);
void process(const MSG&, OUT&);
void send(const OUT&);
void error(const char* what) { throw what; }
while (true)
try
{
MSG msg;
receive(msg);
OUT out;
process(msg, out);
send(out);
}
catch(const char* what)
{
log_error(what);
}
Now... Both snippets end up doing exact same thing, but:
1. try/catch is shorter
2. more importantly, conditional logic is out of sight; first two
variants look convoluted (and in this particular case, they indeed are
convoluted for no good reason).
Your turn to show what you think is not "correct" here.
Goran.
I am just saying one thing that "throwing error is potentially buggy".
Not that it is convenient for an error string to be written to a log
file, nor that it is elegant to mean stack unwind to exit. I am not
even really talking about simplicity neither, even whether it is an
real error or not, but merely correctness of the error reporting
mechanism. Again, I am not talking about any implement of your or his
or her correct throwing strategy, but the prevailing, the frequently
taught, probably the 'standard' one.
Let's first confine errors to only errno-like classes, assume they
all inherit from std::exception.
class E_PERM;
class E_NOENT;
class E_SRCH;
class E_INTR;
class E_IO;
class E_BADF;
class E_AGAIN;
class E_NOMEM;
class E_NOSPC;
class E_INVAL;
class E_PIPE;
class E_TIMEDOUT;
class E_NAMETOOLONG;
class E_FBIG;
class E_LOOP;
.....
// Class String has all similar members as std::string
// except throwing E_... class
//
class String {
// Take one for instance:
// Throw: E_INVAL $index is invalid
// E_FBIG String length too long
// E_NOMEM Not enough memory
//
void insert(size_t index, const String& str);
};
Now we are writting a funcion process_msg. All called functions throw
E_... in exceptional condition.
// Throw: E_INVAL $in is invalid
// E_FBIG result string is too long
// E_NOMEM not enough memory
//
void process_msg(const String& in, String& out);
{
MutexLock lock(mutex); // Not likely to fail. But shouldn't
// a robust program
catch the possible
// E_INVAL, E_PERM,
E_AGAIN from leak?
String s1,s2;
if(in.size()<=0) {
throw E_INVAL;
}
db.read(s1,1000); // may throw E_INVAL, E_FBIG, E_NOMEM,...
parse(s1,s2); // may throw E_INVAL, E_FBIG, E_NOMEM,...
process(s1,s2,out); // may throw E_INVAL, E_FBIG, E_NOMEM,...
}'
Then, a tiny part of the loop in the server-like program:
try { process_msg(in,out); }
catch(E_INVAL& e) {
send("Invalid argument",e ); // peer end could have a wrong message
// Probably one want to log message here, but there are more story
// about stack unwinding that can occur here. If error is
// misinterpreted, the program goes to std::terminate for the good.
// The error report is not just for logging purpose.
// This is one reason a function should be somewhat responsible
// to its declared thrown objects.
}
catch(E_FBIG& e) {
send("Result too long",e); // peer end could have a wrong message
}
This pattern has been explained several times. It should be clear
that at least all process_msg(..) documented errors should be caught
(probably convert to another one) in the function process_msg(..).
The prevailing "catch only those you know how to handle" style and
the "rethrow error" preaches won't work well in such error handling
or message passing scenario.
So the revised version (not the popular style):
void process_msg(const String& in, String& out)
try {
MutexLock lock(mutex); // Note: E_INVAL, E_PERM, E_AGAIN might be
// thrown here, but no
way for a local
// catch. So we added a
try/catch layer.
String s1,s2; // E_NOMEM might be thrown
if(in.size()<=0) {
throw E_INVAL; // this is the responsible one
}
try { db.read(s1,1000); }
catch(E_INVAL&) {
// What to throw? process_msg(..) only documented 3 errors.
// Do we need more errors to report? Yes, so add E_IO to mean
// vague error.
throw E_IO;
}
catch(E_INTR&) {
// Let this be handled correctly
}
catch(E_IO&) {
// E_IO had been decided for a vague error. So this error
// can't be directly reported.
throw E_IO;
}
catch(E_PERM&) {
// process_msg(..) intends to hide the implement detail that it
// reads data from a database or config file. So throw E_IO.
throw E_IO;
}
catch(E_FBIG&) {
// To be defensive. Not knowing how 'db.read(..)' will actually be
throw E_IO;
};
// There are other possible thrown type by definition.
s1+=in; // Lucky, E_FBIG is appropriate
try { parse(s1,s2); }
catch(E_INVAL&) {
throw E_IO;
}
catch(E_NOENT&) {
throw E_IO;
}
catch(E_FBIG&) {
throw E_IO;
};
// There are other possible thrown type by definition.
try { process(s1,s2,out); }
catch(E_INVAL&) {
throw E_IO;
}
catch(E_RANGE&) {
throw E_IO;
}
catch(E_BADMSG&) {
throw E_IO;
}
catch(E_FBIG&) {
throw E_IO;
};
// There are other possible thrown type by definition.
// Can the above annoying catch handlers be saved?
// My answer is no.
}
catch(E_INVAL&) {
throw; // *** BUG ***
//E_INVAL may be from the lock failure. There
// are occasionally similar codes causing such
// conflicts.
};
See, it is not easy for process_msg(..) to be merely responsible for
those documented throw types. To add to this burden, this style
require all functions of the program to follow this coding standard.
Note: Some approaches use specific name space or throw classes, but
they basically only reduce the chance of being misinterpreted, with
lots of unbalanced costs. Still, not the final solution.