5
\$\begingroup\$

std::stoi may throw exceptions so it needs to be surrounded by try/catch.

In applications where std::stoi may be used frequently, it could be useful to have a wrapper.

Is this good practice?

int _stoi(std::string str, int* p_value) {
    // wrapping std::stoi because it may throw an exception

    try {
        *p_value = std::stoi(str);
        return 0;
    }

    catch (const std::invalid_argument& ia) {
        //std::cerr << "Invalid argument: " << ia.what() << std::endl;
        return -1;
    }

    catch (const std::out_of_range& oor) {
        //std::cerr << "Out of Range error: " << oor.what() << std::endl;
        return -2;
    }

    catch (const std::exception& e)
    {
        //std::cerr << "Undefined error: " << e.what() << std::endl;
        return -3;
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ std::strtol addresses the problem in a much cleaner way. \$\endgroup\$ – vnp Nov 1 '18 at 22:26
  • \$\begingroup\$ @vnp -- agreed. And std::stoi is specified as if it was a wrapper around std::strtol that reports errors by throwing exceptions. So the way to not deal with exceptions is not to add another wrapper, but to unwrap and use std::strtol directly, or perhaps through a wrapper that provides the desired (non-exception) interface. \$\endgroup\$ – Pete Becker Nov 2 '18 at 14:39
  • \$\begingroup\$ @vnp Or even the brand new std::from_chars. \$\endgroup\$ – Calak Nov 2 '18 at 20:20
  • \$\begingroup\$ I don't think it's good practice at all, as throw/catch make -fno-exceptions impossible, which defeats the purpose of your code. \$\endgroup\$ – user1095108 Nov 4 '18 at 20:16
9
\$\begingroup\$

This wrapper effectively removes some of the available functionality from std::stoi() because its signature is

int stoi(const std::string& str, std::size_t* pos = 0, int base = 10);

Because your wrapper does not allow a pos or base argument you cannot use it to give you the number of characters processed (with pos) nor to convert using a different base. std::stoi() provides default values for these arguments so you'd only have to provide them if you need the non-default behavior.

Also, you don't take the std::string argument by const reference like std::stoi() -- the string argument is probably not too expensive to copy in this context but why not accept the argument the same way std::stoi() does?

For completeness, I would also implement the overload of std::stoi() which accepts a std::wstring (and possibly std::stol() and std::stoll()).

I would also avoid the leading underscore, as identifiers with a leading underscore are reserved.

With these suggestions the wrapper would be implemented as

int stoi(const std::string& str, int* p_value, std::size_t* pos = 0, int base = 10) {
    // wrapping std::stoi because it may throw an exception

    try {
        *p_value = std::stoi(str, pos, base);
        return 0;
    }

    catch (const std::invalid_argument& ia) {
        //std::cerr << "Invalid argument: " << ia.what() << std::endl;
        return -1;
    }

    catch (const std::out_of_range& oor) {
        //std::cerr << "Out of Range error: " << oor.what() << std::endl;
        return -2;
    }

    catch (const std::exception& e)
    {
        //std::cerr << "Undefined error: " << e.what() << std::endl;
        return -3;
    }
}
\$\endgroup\$
  • \$\begingroup\$ setting pos = 0 yields warning: zero as null pointer constant. I see that this is how std::stoi is declared, but is this safe? \$\endgroup\$ – Sparkler Nov 2 '18 at 15:21
  • \$\begingroup\$ @Sparkler I don't see that warning even when I compile with all warnings enabled. \$\endgroup\$ – Null Nov 2 '18 at 15:28
  • \$\begingroup\$ compiler's indication is that it's coming from [-Wzero-as-null-pointer-constant] \$\endgroup\$ – Sparkler Nov 2 '18 at 16:41
  • \$\begingroup\$ @Sparkler Hmm, do you get the warning even if you just use std::stoi()? \$\endgroup\$ – Null Nov 2 '18 at 16:45
  • \$\begingroup\$ there's no warning when I just used std::stoi() \$\endgroup\$ – Sparkler Nov 2 '18 at 17:39
8
\$\begingroup\$
int _stoi(std::string str, int* p_value) {

Identifiers that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

I would expect the signature of your _stoi to match that of the std::stoi you are wrapping. So you should take the std::string by reference-to-const, take an in-out parameter to indicate how much of str was processed, and the base you are converting to.


    try {
        *p_value = std::stoi(str);
        return 0;
    }
    catch (...) {
        return -3;
    }

Instead of returning integers that represent error-codes, leverage the type-system. There are a class of types called "either" types that let you return a result or an enumerated error. There are also variant types that let you return one of multiple types of return or error values.

If you plan on supporting the full set of std::stoX family of functions and their overloads between std::string and std::wstring, I urge you to look at lippincott functions as well as variadic argument passing.

\$\endgroup\$
8
\$\begingroup\$

Your function signature is misleading. You give almost the same than the std version. It's not obvious that the return type isn't the result.

int _stoi(std::string str, int* p_value)
 // std version, where size_t is an unsigned int
int stoi( const std::string& str, std::size_t* pos = 0, int base = 10 );

So both can be called from a string and a ptr to an (unsigned) integer and return an integer.

  • identifiers beginning by an underscore are reserved. If you want to avoid name collisions, you can (and should) enclose your function in a namespace.
  • Don't pass your string by value, instead get a const& or a string_view
  • Instead of magical return values, you can return an enum.
  • Or, even better, you can, maybe, wrap the return value in a sum type like expected or optional.
  • Here, if user don't want exceptions, he still have to pay for it, you are just simply hiding them. Your code isn't "exception-free", but "exception-safe".
  • It would be better if internally you do the work in a exception-safe way. So, people who deactivated exceptions can still use your function.
  • Which advantage versus std::from_chars?
\$\endgroup\$
  • \$\begingroup\$ Can you please explain/add link explaining "wrap the return value in a sum type like expected or optional."? \$\endgroup\$ – Sparkler Nov 2 '18 at 15:12
  • \$\begingroup\$ A std::optional<T> is a type which may contain a T value, or not. Look these two link to understand usage and purpose Bartek's coding blog and Modern C++. \$\endgroup\$ – Calak Nov 2 '18 at 15:29
  • \$\begingroup\$ std::expected<T, E> is a type which hold a T value, or an error E. A nice article about using expected-like type. You have a lot of implementations on github \$\endgroup\$ – Calak Nov 2 '18 at 15:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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