Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception: error code, message #12

Merged
merged 2 commits into from
Jan 3, 2014
Merged

Conversation

jarlela
Copy link
Contributor

@jarlela jarlela commented Jan 3, 2014

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

  • The error code returned by the netcdf-c methods is included as a member variable in NcException
  • The exception message is now retrieved from the netcdf-c api or strerror
  • Exception messages are passed to the NcException ctor as char pointers in order to avoid in-place conversion to std::string which may throw

@ghost ghost assigned russrew Jan 3, 2014
@russrew russrew merged commit 951ca46 into Unidata:master Jan 3, 2014
@jarlela jarlela deleted the exception_code_msg branch January 10, 2014 11:15
@jarlela
Copy link
Contributor Author

jarlela commented Jan 6, 2016

@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
Copy link

citibeth commented Jan 6, 2016

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
#12 (comment).

@jarlela
Copy link
Contributor Author

jarlela commented Jan 6, 2016

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), ... );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants