Unidata / netcdf-cxx4

Official GitHub repository for netCDF-C++ libraries and utilities.
Other
124 stars 49 forks source link

Exception: error code, message #12

Closed jarlela closed 10 years ago

jarlela commented 10 years ago

These are the changes apart from the change to the hierarchy of exception classes included in #9

jarlela commented 8 years ago

@citibeth To avoid unnecessary in-place conversion to std::string, e.g. from all the static char arrays in ncCheck. In principle the string construction could also throw and I think it makes sense to not demand that the argument is passed as a string&.

citibeth commented 8 years ago

Hello Jarle,

Thank you for your reply. I had not thought through these issues.

With C++11, maybe move constructors are the right approach:

http://stackoverflow.com/questions/15831029/not-using-stdstring-in-exceptions

C++11 make this much easier by using move constructors (instead of copy constructors) where possible. Since the move constructor for std::string never throws exceptions you can safely use a std:string in your exception type as long as you properly implement move constructors, and ensure that they are used. Note that the initial construction of the object to be thrown in a throw expression is NOT part of the exception throwing process, so that constructor can throw an exception without causing a double exception (and terminate()).

For example: MyExceptionConstructor(std::string &&message, char *file, int line)

I suppose we don't want to lock this library into C++11 quite yet. But maybe add #ifdef versions of the constructors that use move semantics if a C++11 compiler is detected? Use of char * really needs to be deprecated, and it makes it harder for the user to be writing ".c_str()" all over the place.

Your thoughts? -- Elizabeth

On Wed, Jan 6, 2016 at 4:30 AM, Jarle Ladstein notifications@github.com wrote:

@citibeth https://github.com/citibeth To avoid unnecessary in-place conversion to std::string, e.g. from all the static char arrays in ncCheck. In principle the string construction could also throw and I think it makes sense to not demand that the argument is passed as a string&.

— Reply to this email directly or view it on GitHub https://github.com/Unidata/netcdf-cxx4/pull/12#issuecomment-169276079.

jarlela commented 8 years ago

I think the constructors using const char * arguments should remain, as these are needed to avoid unnecessary std::string construction when calling with string literals (e.g. throw MyExceptionConstructor("My error", ...)). Move constructors in std::string would not help in this regard.

To avoid .c_str(), I think including constructor overloads taking a const reference would be a better solution than relying on move semantics. Using move semantics, a user having a std::string lvalue would need to call std::move to pass the string to the exception constructor:

using namespace std;
string myMessage = "My error";
throw MyException(move(myMessage), ... );