1
\$\begingroup\$

I am using Boost for reading a config file that is in the .json format. I am using a class to do this and I am calling exit(message) in case of bad values or default value gotten by Boost get() function.

The class is here:

class ConfigFile
{
private:
  PTree m_ptConfig;  

  void failedGetValue(const std::string& message)
  {
    std::cout << message << std::endl;
    //TODO throw exception
    exit(EXIT_FAILURE);
  }

public:
  ConfigFile(const std::string& name);
  ~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";
      failedGetValue(msg);
    }

    return val1;
  }
  double getVal2() {/*the same as before*/};
  // ... more values the same way
};

If I want to throw exceptions like MyConfigFileException instead of calling exit:

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();
  }  
};

How would you suggest I do this? Shall I just throw it? Shall I catch where the getValX() function is called or I shall catch it in main()? I am new to exceptions.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Throwing

Yes, just throw it:

if (val1 < 0)
  throw MissingConfigValueException("getVal1() failed or val1 < 0");

Don't print an error message before throwing. Printing is left to whoever catches the exception. For many exceptions (such as missing config values), there may be nothing wrong, so no need to print anything.

When you do print error messages, print to std::cerr — error messages belong on standard error, not standard output.

failedGetValue is not needed, since all it would do is throw an exception.

Catching

Catch exceptions where you can handle them. If the code that calls getVal1 can recover from a missing config value (perhaps by using a default), then it should catch the exception and proceed:

try {
  foo = config.getFoo();
} catch (MissingConfigValueException &e) {
  foo = DEFAULT_FOO;
}
doSomethingWith(foo);

If the caller can't handle the exception, then don't catch it. Just let it propagate up automatically. You don't have to do anything; that's the point of exceptions.

You don't have to catch all exceptions in main, because the default handler should print the exception and exit. You might do this if you want to handle them differently — for example, if you want to abort an operation rather than exiting, or if you want to log exceptions to somewhere other than standard error.

try {
  do(stuff);
} catch (std::exception &e) {
  std::cerr << "Uncaught exception: " << e.what() << std::endl;
}

Comments on other aspects of the program

If getVal2 is just like getVal1 with a different path, then they should be calls to one function, with the varying parts as parameters:

double getVal1() const { return getVal("path.to.val1"); }
double getVal2() const { return getVal("path.to.val2"); }

Or, even simpler, call getVal directly and don't bother with a separate function for each value.

Two problems with the name MyConfigFileException:

  • Names generally shouldn't contain My. Everything is MyFeature to someone, so it conveys no useful information. The name of a class should say what its instances represent, not who wrote it.
  • It doesn't say what the problem is. Exception names should say what's wrong, not just which component had a problem. In this case the problem is that a config value is either missing (MissingConfigValue) or invalid (InvalidConfigValue).

It would be nice to distinguish missing values from invalid ones, perhaps using ptree::get_optional. That would let you use the default (when there is one) in place of missing values, but reliably detect invalid values.

MyConfigFileException should derive from std::runtime_error, since it indicates a problem with the environment (specifically the config file), not a bug in the program. Actually, in a small program, I wouldn't bother with a new exception class — I'd just throw a runtime_error with an appropriate error message.

PTree does all this for you

Actually, you needn't check for missing values or throw an exception yourself, because ptree already does it.

The one-argument form of ptree::get throws an exception when the value is missing, and the two-argument form takes a default which is returned instead. This is exactly what you want for config values that have a default: get<int>("foo.bar", 5) returns the configured value of foo.bar, or 5 if there isn't one. When there is no default, get<int>("foo.bar") returns the configured value, or throws an exception.

ptree is intended for storing things like config files, and it's designed to make this convenient. Its authors have done your work for you!

\$\endgroup\$
2
  • \$\begingroup\$ About get, I was using it for display a message about the value, because json has no comments, is this wrong? \$\endgroup\$ May 22, 2014 at 7:20
  • \$\begingroup\$ I don't understand. Do you mean you wanted to throw a different exception with a better error message? That's fine; just catch their exception and throw yours, or use ptree::get_default to see if the value is present. \$\endgroup\$
    – Anonymous
    May 22, 2014 at 13:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.