debugcppMinor
Error-Handling Class
Viewed 0 times
handlingerrorclass
Problem
I have been doing some searching into how to properly handle errors in C++. I have found that the two most common techniques are exceptions and enumerations. I have created a simple class to expand on the enumeration method. It's main features are boolean operator overloading to return if the object actually has an error "attached to it", and the ability to set messages at the point where the error occured. It uses an enumeration so that default error messages can be set. The ones in my code are for my current program, and are meant to be examples. Here is my code:
```
enum Err_Type {
NO_ERR, EMPTY, INVALID_INPUT, OUT_OF_RANGE,
INVALID_FUNC_NAME, INVALID_PARANS, INVALID_OPERATOR, DIV_ZERO, INVALID_BASE,
FILE_IO_ERROR, CUSTOM
};
class Error
{
public:
// Constructors
Error() { Type = NO_ERR; Message = ""; }
Error(Err_Type type) { Type = type; SetMessage(type); }
Error(std::string msg) { Type = CUSTOM; Message = msg; }
// Operator Overloading
explicit operator bool() const { return Type != NO_ERR; }
bool operator !() const { return Type == NO_ERR; }
void operator ()(Err_Type type) { Type = type; SetMessage(type); }
// Public Methods
void ChangeMessage(std::string msg) { Type = CUSTOM; Message = msg; }
```
enum Err_Type {
NO_ERR, EMPTY, INVALID_INPUT, OUT_OF_RANGE,
INVALID_FUNC_NAME, INVALID_PARANS, INVALID_OPERATOR, DIV_ZERO, INVALID_BASE,
FILE_IO_ERROR, CUSTOM
};
class Error
{
public:
// Constructors
Error() { Type = NO_ERR; Message = ""; }
Error(Err_Type type) { Type = type; SetMessage(type); }
Error(std::string msg) { Type = CUSTOM; Message = msg; }
// Operator Overloading
explicit operator bool() const { return Type != NO_ERR; }
bool operator !() const { return Type == NO_ERR; }
void operator ()(Err_Type type) { Type = type; SetMessage(type); }
// Public Methods
void ChangeMessage(std::string msg) { Type = CUSTOM; Message = msg; }
Solution
Orthography
I guess it is a typo, but
The message for
Style
Usually, it is preferred to have every enumerator on it's own line. This also applies to methods, meaning that you should have every statement on it's own line, e.g. your
Also, you should start indenting your methods. Everytime you enter a new block (= write a new
(Also applies to all other methods)
Another important point is your naming. Although there are no fixed rules for this in c++, you'll usually see member variables, methods and class names beginning with a lowercase letter. Also, if you have compound names, most c++ programmers prefer the so called snake_case style, meaning that each part of the name should be written in lowercase and joined with an underscore (this style is used throughout the standard library, but there are other possibilities, such as camelCase, if you are uncomfortable with it). Most important, however, is that you stay consistent. Thus
Since
Improvements
Since you tagged this question with c++11, I advise you to use
Since this class handles errors, printing to
Furthermore, the method
I guess it is a typo, but
INVALID_PARANS should probably be called INVALID_PARAMS.The message for
INVALID_PARANS also contains the word "parantheses", which ought to be "parentheses".Style
Usually, it is preferred to have every enumerator on it's own line. This also applies to methods, meaning that you should have every statement on it's own line, e.g. your
ChangeMessage method should become something likevoid ChangeMessage(std::string msg) {
Type = CUSTOM;
Message = msg;
}Also, you should start indenting your methods. Everytime you enter a new block (= write a new
{) you should go one level of indentation deeper, i. e. add a tab or 2, 4, or 8 spaces (according to your personal preference). This means that the SetMessage method should look more like this:void SetMessage(Err_Type type) {
switch (type) {
case EMPTY:
Message = "Entry cannot be empty!";
break;
...
}
}(Also applies to all other methods)
Another important point is your naming. Although there are no fixed rules for this in c++, you'll usually see member variables, methods and class names beginning with a lowercase letter. Also, if you have compound names, most c++ programmers prefer the so called snake_case style, meaning that each part of the name should be written in lowercase and joined with an underscore (this style is used throughout the standard library, but there are other possibilities, such as camelCase, if you are uncomfortable with it). Most important, however, is that you stay consistent. Thus
Err_Type should either be called err_type or errType in line with the style you choose.const-CorrectnessSince
GetErrMessage and DisplayMessage do not alter any member variables, they should be marked const.Improvements
Since you tagged this question with c++11, I advise you to use
enum class instead of enum to keep your global scope clean, since enum just puts all enumerators into global scope. Also, your implementation currently lacks a method to get the the actual Err_Type that an Error object represents.GetErrMessage returns a string by value, which means making a copy of it everytime the method is called. Most of the time, returning a const std::string& would be the better choice here. Since this class handles errors, printing to
std::cerr instead of std::cout would be the appropriate behaviour for DisplayMessage. However, this method shouldn't actually exist in the first place, since it violates the SOLID principles. What if somebody wants to output an error into a file? Bad Luck. Also, it is highly discouraged to use std::endl in most cases, since it not only writes a line terminator but also flushes the buffer, which can lead to severe performance loss (assuming that the endl in your code is indeed std::endl).Furthermore, the method
ChangeMessage is a bit misleading: It does not only change the message, but it also silently changes the error type to CUSTOM. You should either change your method to allow custom error messages for all kinds of errors or rename it to reflect what it does.Code Snippets
void ChangeMessage(std::string msg) {
Type = CUSTOM;
Message = msg;
}void SetMessage(Err_Type type) {
switch (type) {
case EMPTY:
Message = "Entry cannot be empty!";
break;
...
}
}Context
StackExchange Code Review Q#157448, answer score: 5
Revisions (0)
No revisions yet.