Exception-handling optimisation

Question

I have posed another question here and I am writing this one for some code review:

class ConfigFile { private: PTree m_ptConfig; public: ConfigFile(const std::string& name) { json::read_json(name, m_ptConfig); } ~ConfigFile(); double getVal1() { std::string path = "path.to.val1"; double val1 = m_ptConfig.get<double>(path, -1); if (val1 < 0) { std::string msg = "getVal1() failed or val1 < 0"; throw MyConfigFileException(msg); } return val1; } double getVal2() {/*the same as before*/}; // ... more values the same way }; 

The exception class is here:

class MyConfigFileException : public MyIOException // MyIOException is derived from std::exception { private: std::string m_msg; public: MyConfigFileException(const std::string msg) : msg(msg) {} virtual const char* what() const throw() { return m_msg.c_str(); } }; 

The handling for this kind of exception I have done it in main function:

int main(int argc, char **argv) { const std::string configFileName = argv[1]; try { RunAlgo algo(configFileName); algo.runAlgorithm(); } catch (MyConfigFileException& mcfe) { std::cout << "Config File Exception: " << mcfe.what() << std::endl; return -1; } catch (MyIOException& mioe) { std::cout << "IOException: " << mioe.what() << std::endl; return -2; } catch (std::exception& e) { std::cout << "Unknown exception: " << e.what() << std::endl; return -3; } return 0; } 

I use the chatch blocks to see if it has crashed because of me or not, that is why I just print the message and return a value. (Maybe this is not necessary since it returns a different value, but I let it there for the moment)

In the runAlgorithm() function I have a loop, where I treat another kind of exceptions:

// ... void RunAlgo::runAlgorithm() { ConfigFile cf("config.json") double v1 = cf.getVal1(); // ... for (int i = 0; i < 100; i++) { // ... try { foo(cf); } catch (MyOtherExceptions & moe) { std::cout << moe.what() << std::endl; } // ... } // ... } 

MyOtherExceptions are going to be treated separately, like: for the current iteration, there is a problem with a buffer, send a message to a queue.

foo is implemented here:

void foo(const ConfigFile& cf) { double v2 = cf.getVal2(); // some other code that may throw MyOtherException exceptions } 

Do you think I am using the exceptions correctly?

---

I admit that the handling is not really done, but I am asking if I am doing right with the throws and try-catch blocks, supposing that instead of just printing messages the handling is done (as "sending message to a queue").

Answer 1

That is not really exception handling:

In your main() method:

You are branching on three different types of exceptions, but your action is the same: Print a message. Doing so is fine, but the point for different catch blocks is not obvious to me. Anyway:

1. Use std::cerr instead of std::cout for errors.
2. return 1 (or any value different from 0) if your program fails.

In runAlgorithm()

Generally you should only catch exceptions you want to handle. And printing a message is not really handling an exception, that is more like cheap debugging.

You are catching the MyOtherException exception and ignoring it. Why? Is it irrelevant to the algorithm? If so, why print a message? If not, why continue the algorithm? You might want to move the catch block out of the loop if it is fatal, or possibly remove it at all if you can't deal with it.

As a rule of thumb: Only catch an exception if

1. you can do something about it, this might be trying a different way to achieve the goal of your function
2. you want to wrap it inside a more appropriate exception, for example you could catch a low level io exception and wrap it in a parse exception explaining in more detail what went wrong
3. it is irrelevant to the calling function
4. it is the main method. C++ behaves ugly on uncaught exceptions. But you should probably return 1 instead of 0.