0

I'm writing a C++ class. Some of its fields are STL containers, some aren't. While writing the methods I began wondering, how should I handle invalid values passed to methods? For example, some methods are more or less wrappers to STL container methods. Many STL methods just have "undefined behavior" when invalid iterators are passed. I guess it's like that because it allows the STL code to ignore these cases and thus be faster.

But for higher level code, what should I do? I do throw exceptions when an unexpected error occurs, for example an error made by a mistake a developer made. But in this case the parameter value depends on the interface user, not on the implementor. I could ignore invalid parameters and invalid iterators, etc. and "pass" the problem to lower-level functions, which would then have undefined behavior, but I could also throw an exception or at least find some way to report an error.

What would be the best thing to do?

Example: I have a class representing a tree node and it has a method add_child() which takes a std::shared_ptr parameter. Should I check the value or let the user make sure a nullptr isn't passed? Or for an invalid iterator, pass it to STL methods or report an error? If I should report - are exceptions the right solution?

cfa45ca55111016ee9269f0a52e771
  • 2,153
  • 3
  • 26
  • 39
  • Kind of an open ended question with no 1 correct answer. Try asking this on: http://programmers.stackexchange.com/ – zdan Feb 08 '13 at 17:53
  • I share zdans opinion... there is always SEGV as a fallback ;-) (and easy to code too) – Najzero Feb 08 '13 at 17:59

2 Answers2

2

I think a universal answer cannot be given. Too many things affect the decision that you haven't mentioned: first of all, are you developing a library or an application?

In the first case, flexibility should be favored and the best choice is usually to design your interface in a way that leaves the decision up to the client. Since you don't know what clients will use your class and which requirements they will have, it's best to make as few assumptions as possible. If your clients have tight performance requirements, they won't be willing to pay the performance penalty of an extra bounds check, for instance. This is, basically, the reason why bound checks are not performed by STL either.

In the second case, it depends on the performance requirements of your application and the exception safety guarantees your class has to provide. Which operations need to be no-throw? Are you allowing or planning to use your class inside an STL container? Is it going to be used in the most critical part of your application, so that a small performance performance gain in the algorithms of this class will affect the overall execution time of your application?

There is no such a thing as a "universally best design". Engineering is all about finding the right compromise according to the particular situation at hand, and software engineering is no exception to this.

Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
  • I see. If I decide to report an error, how do I report? I mean, there are so many options... for example, one that I'm not sure about: When should I use exceptions? In cases I made a mistake, assert does the job, doesn't it? If case the user passes a null pointer it's something else, it's unexpected. Should I throw an exception? – cfa45ca55111016ee9269f0a52e771 Feb 08 '13 at 19:38
  • @fr33domlover: It seems to me you're asking the same question I answered :-) There is no way to tell in absolute without knowing your requirements in terms of performance, design flexibility, etc. – Andy Prowl Feb 08 '13 at 19:42
  • You are saying that there is not a universal answer but the only possibility you suggest is to do no check at all (like STL library). I would say that an assertion is better than no check at all. – Emanuele Paolini Feb 12 '13 at 08:55
  • @manu-fatto: There's no single sentence in my answer where I suggest that "the only possibility is to do no check at all". If you need a reason to downvote my answer, consider finding a better one. – Andy Prowl Feb 12 '13 at 12:58
  • You say that one possibility is "to do no check" (which I agree could be the best solution in some situation) but you don't suggest any other possibility even if you are saying that there *are* other possibilities. – Emanuele Paolini Feb 12 '13 at 15:07
  • @manu-fatto: That's very different from saying that "the only possibility is to do no check at all". The other possibilities are obvious: the OP himself is mentioning them, including return an error code, throw exceptions, etc. – Andy Prowl Feb 12 '13 at 15:09
  • @Andy: if you read carefully my comment I say "the only possibility you suggest" and not "you suggest ther is only one possibility". But this is not the point... the point is that the other possibility you implicitly suggest (i.e. raising an exception or return an error code) are, in my opinion, not appropriate to catch a mistake done by the developer. – Emanuele Paolini Feb 12 '13 at 15:38
  • @manu-fatto: *Error reporting* in general is not just about developer mistakes you make writing a routine. Assertions are supposed to check the *internal* functioning of a routine, i.e. a mistake *you* made in programming it. If an assertion is triggered, your routine contains UB. This is not necessarily true of an out-of-bounds index, which comes from the *client* of your routine: the client might still be able to handle this error condition. You just need a way to report it. And assertions are completely ignored in release, so you're not reporting anything. – Andy Prowl Feb 12 '13 at 15:43
  • The client code is also written by a developer. The poster is saying that he would manage "invalid values passed to a method" and as an example says "a developer mistake". You don't catch exceptions (or check error values) to detect if you are using a method in the incorrect way. At least this is my opinion. I see that our discussion is detected as "too long"... I'm interested in your opinion but maybe it's not the case to continue further. – Emanuele Paolini Feb 12 '13 at 16:04
-1

In such cases I would use assertions as in:

assert(i < vec.size());
return vec[i];

This way if the program is compiled with the NDEBUG flag you get the maximum performance and no check. While developing you can get errors at runtime and debug your code. The assert macro is available with #include < cassert >.

Emanuele Paolini
  • 9,912
  • 3
  • 38
  • 64
  • This is probably good if we're talking about an internal function that only your own code calls. If this is an interface method to some other client, you don't want to trip an assertion. An assertion says "I made a mistake," not "you made a mistake." You shouldn't have an assertion failure in production code. – JohnMcG Feb 08 '13 at 18:23
  • 1
    @JohnMcG An assertion means that the internal state in the procedure is incorrect, and that anything you do from that point out is undefined behavior. There are cases where might want to try to carry on, but they are rare. In most cases, violating a pre-condition of a function, for example, should result in an immediate termination of the program, executing as little additional code as possible (so no stack walkback, and no destrutors). – James Kanze Feb 08 '13 at 18:48
  • @JohnMcG: I interpret an assertion as "a mistake had happened". What are the drawbacks of using assertions to check if the client is using library code in a wrong way? – Emanuele Paolini Feb 09 '13 at 06:59
  • If you trip an assertion, the program will end, and the user will see the message, "assertion failed, x != 0," or whatever the assertion condition is. The client developer has no control over it. An exception, on the other hand, would give the client developer the ability to catch and handle it. One other problem with the assertion approach is that many compilers turn off assertions in release builds. – JohnMcG Feb 11 '13 at 23:11
  • An error in the developer code is not an exception. It should be catched in the development process. Hence an assertion is appropriate. In release code the assertion is excluded in the compilation process so that the code is as fast as possible. – Emanuele Paolini Feb 12 '13 at 07:07