4

We have a large code base with a lot of code which does this kind of thing:

bool DoSomething(CString Value)
{
   if(Value == "bad")
   {
       AfxMessageBox("description of error");
       return false;
   }
   return true;
}

or even just this:

bool DoSomething(CString Value)
{
   if(Value == "bad")
   {
      return false;
   }
   return true;
}

We have considered various alternatives:

  • Exceptions - Basically we’re sold on the cons as described here: http://www.codeproject.com/Articles/38449/C-Exceptions-Pros-and-Cons
  • Passing an additional string ref parm which is populated with error text - but it requires a pre-instantiation of the error string prior to the call, and adds bulk to the parm lists
  • Populating a ‘last_error’ member variable - this seems to suffer (imo) the drawbacks expressed here: Is GetLastError() kind of design pattern? Is it good mechanism?
  • Passing back an enum (or the like) which can be mapped to a description of the error with some other function - but this feels ‘heavy’ for small functions and objects, and also creates space between the place the error occurs and the place where the messages are maintained (though I suppose in multi-ligual environment they’d appreciate the centralization of the text).

So I wondered if we could create a set of classes like the following:

class CResult
{
protected:
   CResult()
   {
      // Don't have to initialize because the derived class will do it
   }
public:
   operator bool() { return m_Result; };

   bool m_Result;
   CString m_Message;
};

class CSuccess : public CResult
{
public:
   CSuccess()
   {
       m_Result = true;
   }
};

class CFailure : public CResult
{
public:
   CFailure(const CString & Message)
   {
      m_Result = false;
      m_Message = Message;
   }
};

Then the above code could look like this:

CResult DoSomething(CString Value)
{
   if(Value == "bad")
   {
      return CFailure("description of error");
   }
   return CSuccess();
}

What I like about it:

  • The code is still readable and the error message is maintained near the error condition
  • The programmer will be slightly more compelled to actually provide an error string on an error condition (yes, they could provide blanks but that would seem a more egregious mistake, imo)
  • The caller doesn’t have to create any specialized variables prior to the function call, and can still treat the function result like a bool - or in a case where the error is not ignored, easily retrieve an explanation
  • The next programmer can what error model is in use just by looking at the function definition

The main drawback I see is that there is higher overhead on success, in that an object and a string and a bool will all be instantiated - but a lot of the times in our app the code in question is not performance sensitive, such as validating user input, etc.

Am I missing some other big drawback? Is there an even better solution?

Community
  • 1
  • 1
hunter
  • 307
  • 3
  • 11
  • 6
    *Is there an even better solution?* Yes, use exceptions. – Praetorian Jul 06 '12 at 16:29
  • 2
    I'd suggest you reconsider exceptions. The only real issues are static data size, and having to use exception-safe practices (RAII in particular) - but to me, the latter is very much an advantage. (Unless your compiler doesn't support zero-cost exceptions, in which case the performance impact might be an issue if you're not careful). – Mike Seymour Jul 06 '12 at 16:33
  • Exactly. Use exceptions. This looks like the kind of language I spend my time coding in: C+ (C++, but with one less plus). And I'm using the low level POSIX API. If you're doing real C++ **use exceptions**!!! – Linuxios Jul 06 '12 at 16:33
  • 4
    And as for those cons, not doing something because it might be harder but better is no reason, and resources? Any self-respecting C++ program will use smart pointers, invalidating their claim that exceptions create resource leaks. And how about `finally` blocks? – Linuxios Jul 06 '12 at 16:36
  • @linuxios: exceptions + smart pointers = slow and buggy. – Rafael Baptista Jul 06 '12 at 16:41
  • 7
    @RafaelBaptista: Manually checked return values + manually managed raw pointers = slower and buggier. – Mike Seymour Jul 06 '12 at 16:43
  • 6
    @RafaelBaptista given that exceptions should only occur in exceptional conditions, your comment makes no sense. Code is rarely optimized for speed on an exception path because those are not intended to occur regularly. If your code is throwing a lot of exceptions it probably is buggy. If you are using exceptions for control flow you're doing it wrong. – Dave Rager Jul 06 '12 at 16:45
  • 4
    @RafaelBaptista: No. Exceptions + boost::shared_ptr = all memory freed at the right time. – Linuxios Jul 06 '12 at 16:46
  • I was hoping more for some insight on the proposed solution rather than a debate on exceptions - can I reframe the question to obtain that? We've got hundreds of thousands of lines of code written without exceptions in mind: am I wrong to think it would be a nightmare try to go back and make it exception friendly? – hunter Jul 06 '12 at 17:05
  • Lol. manipulating reference counts on every pointer dereference is not free. Now add multi-threading and you have to mutex protect all those smart pointers. Welcome to deadlocks and debugging hell. – Rafael Baptista Jul 06 '12 at 17:08
  • @RafaelBaptista: So, assuming you actually need a smart pointer at all, use `unique_ptr`, which isn't reference-counted. And if you really need multi-threaded reference counting (which probably means you're doing something wrong), then use lock-free counters. Even then, they're not manipulated on "every dereference", just when you create and destroy references. And hopefully there's no need to debug it, since whoever wrote your standard library implementation has already done that for you. – Mike Seymour Jul 06 '12 at 17:16

1 Answers1

4

It can be useful to separate "errors" in two categories:

Fatal errors

These are the kind of errors from which a recovery make no sense.

void check(bool cond, const string& msg)
{
  if (!cond)
  {
    // eventually log it somewhere
    std::cerr << "Fatal: " << msg << std::endl;
    exit(1);
  }
}

Exceptional errors

These are the kind of errors that you can recover from and keep the program in a running state.

void check_ex(bool cond, const string& msg)
{
  if (!cond)
  {
    // eventually log it somewhere
    throw std::runtime_error(msg);
  }
}
Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
Gigi
  • 4,953
  • 24
  • 25