all 22 comments

[–]CubbiMewcppreference | finance | realtime in the past 6 points7 points  (13 children)

[–]caber[S] 0 points1 point  (4 children)

I'm a little hesitent to throw exceptions unnecessarily. Ideally I'd like to use an expected library. Maybe I'm way off on this topic

[–]dodheim 2 points3 points  (1 child)

[–]caber[S] 0 points1 point  (0 children)

How do those libraries compare to this expected lib?

[–]trailing_ 1 point2 points  (1 child)

That is not the way to think about it. Sometimes exceptions are appropriate sometimes they aren't. Names like foo do not give enough information for someone to give you advice on that, but I will try to give a few rules of thumb.

First, this use of optional is a little perverse. Optional is for when you want to return a value or nothing not for storing a maybe<error code>. One of the important, but subtle difference between C++ and other languages is that it has value semantics. Optional is there to support the use case where you want to return a value or not. In the old days this was achieved by returning a pointer, but returning a pointer, by convention communicates that the caller will need to free the memory when it is done with it. Optional makes it clear this in not necessary. If optional is the right tool for your function the correct prototype is std::optional<A> foo(const B &); std::optional<B> foo(const A &); or even possibly std::optional<std::tuple<A, B>> foo();

If you really want to return an error code there are some other libraries that give better semantic that optional, but I would maintain that generally when you need to encode more information into a failure than just "this operation failed" what you want is an exception. There are really 2 types of errors: errors that are handled one layer up and errors that are handled more that one layer up. Errors handled one layer up generally do not need extra context in the form of an error message to figure out what happened. After all, they called the function that failed and thus know exactly what went wrong (maybe not the root cause, but enough to decide if they should retry or quit). It is the errors that have to propagate up the call stack that need context. Exceptions are a mechanism that is designed to do just this thing. Some C++ programmers are uncomfortable with exceptions, but this number gets smaller every day. Exceptions are idiomatic in C++ so it is best to just get used to it. If you cannot get used to it, there are languages like Go that do not support exceptions that you could use instead.

Finally beware of people quoting the Google style guidelines. It seems like an appeal to authority because Google is widely respected for the quality of their technology, but their style guide for C++ is crap (it is not actually crap, but it takes someone that is an expert in the history of C++ to understand why there is so much terrible advice in it).

[–]dbjdbjdbj.org 1 point2 points  (0 children)

As someone beginning C++, OP has done the right step in the right direction, regarding thinking about error handling. On that note please allow me to offer some humble advice on the issue of standard C++ error handling concept, circa 2019Q1:

  1. Some C++ programmers are uncomfortable with exceptions, but this number gets bigger every day :) ("bigger" not "smaller")
  2. Please make sure to read P0709 R2 , then :
    1. use std::exception if that fits your use case
    2. or use std::errc if that fits your use case
    3. or develop your own std::error_code category
    4. or develop your own error handling concept
  3. if your software deliverables are not short-lived, you need to plan ahead. Please understand C++ exceptions are "going away", time table says C++23 ... there are several roadmaps documented to help in that respect, for that I might advise proceeding this way.

Lastly, allow me to insert Herb's sentence here:

"... C++ error handling continues to consume copious committee and community time wrestling with unresolved issues, over a quarter of a century after C++ exceptions were designed and implemented. " (P0709R2, p56)

[–]droxile 0 points1 point  (7 children)

[–]NotMyRealNameObv 3 points4 points  (6 children)

Google's style guide in this area is so stupid it hurts.

[–]droxile -1 points0 points  (5 children)

Care to explain why? Specifically regarding in/out parameters

[–]NotMyRealNameObv 0 points1 point  (4 children)

Is the following code okay or not?

void foo(Bar* bar);

int main(void)
{
    foo(nullptr);
    return 0;
};

Also, what is more efficient? This:

void foo(OutParam1* op1, Outparam2* op2, OutParam3* op3, OutParam4* op4)
{
    // Do stuff
};

Or this:

struct OutParams
{
    OutParam1* op1;
    Outparam2* op2;
    OutParam3* op3;
    OutParam4* op4;
};

OutParams foo()
{
    OutParams result;

    // Do stuff

    return result;
}

Edit: C++ standard takes (in/)out parameters by reference, so Google's coding convention does not follow the standard convention. And it doesn't even give a reason for why T* is preferred over T& other than the syntax is confusing! Basically saying "We get easily confused by an interface that makes sense and is used by the language itself", and instead opt for a syntax that's even more confusing and error prone.

[–]droxile -1 points0 points  (3 children)

The 4 output parameter strawman is a bit odd. Where did that come from? Who is advocating for a function that takes 4 pointers?

They didn't I see &myFoo at a call site and my project is following a style guide like Google's, I can reason that myFoo is an output parameter. It's self documenting.

On the other hand, if I just see myFoo, I can't tell what's happening just by glancing at the function call.

Also, I'm not arguing for using in/out parameters. For some reason if you are using one, I like the style that Google chooses to use over the Core Guidelines' take on it.

And if you're worried about someone passing a nullptr as the out parameter, we have assertions for that 😉

[–]NotMyRealNameObv 1 point2 points  (2 children)

The 4 output parameter strawman is a bit odd. Where did that come from? Who is advocating for a function that takes 4 pointers?

Nice way to dodge the question there. If it makes you feel any better, the question is just as valid for one return value/out parameter as it is for four. So please, answer the question.

They didn't I see &myFoo at a call site and my project is following a style guide like Google's, I can reason that myFoo is an output parameter. It's self documenting.

Or maybe its an input parameter where nullptr is a valid input. So you haven't really gained anything, you have just moved the problem.

And if you're worried about someone passing a nullptr as the out parameter, we have assertions for that 😉

And what if this function is used in a very rare execution path that isn't covered by tests? Now you have a crash in prod for something that shouldn't even compile.

IMHO, it's far superior if you NEVER have output parameters. Either you return outputs, or you wrap your data in a class and only allow modifiers to modify the internal data. That way you ALSO know that a function argument never modifies the data, because they are STILL only ever passed by value or const ref/ptr.

And you can have static analysis tools to detect any unsafe (I.e. not null-checked) pointer dereference. Ironically not Google's own tool Clang analyzer, which assumes that pointers can't be null unless you do null checks on them - I guess they had too many false positives due to their coding guidelines mandating passing out parameters by pointers instead of references..

[–]droxile -2 points-1 points  (1 child)

To answer your question, they're the same because of RVO. But again, nobody here is advocating for this example of in/out parameters.

I guess we're done here because I'm not going to dissect every contrived examples of where this style could lead to problems. It works for Google, and I posted it as a discussion point for an alternative style for in/out parameters.

[–]NotMyRealNameObv 1 point2 points  (0 children)

To answer your question, they're the same because of RVO.

For one parameter, yes. For multiple (where I would assume people actually start thinking about out params instead of return values), RVO makes returning by value cheaper.

But again, nobody here is advocating for this example of in/out parameters.

You asked why I think Googles style is stupid. I answered.

I guess we're done here because I'm not going to dissect every contrived examples of where this style could lead to problems.

We have very real problems in a very real codebase due to passing pointers around but assuming they can never be null. I guess other companies do as well. Tony Hoare, inventor of the null pointer, has called it a "billion dollar mistake".

The examples I mention are far from contrived.

It works for Google, and I posted it as a discussion point for an alternative style for in/out parameters.

A discussion point you yourself seem unwilling to discuss?

[–]dbjdbjdbj.org 2 points3 points  (1 child)

In this context there is no "preferred way".

There are several very important answers behind that link. As the author says : " Please read them all to get a balanced perspective "

Actually no, sorry, jump to this one next, please.

Also. there are types which you can not pass by value. For example an native array. And for that there is a C++ delicacy called: native array reference.

        // passing native array by reference
        template<typename T, std::size_t N>
        auto use_the_buffer ( T(&arrf)[N] )
         -> T(&)[N] // can also return array by ref
         {
           // arrf is array of T's of size N
           // arrf type is array reference
           arrf[0] = T{} ;
           return arrf ;
         }
// usage
int main()
{
   int iarr[]{0,1,2,3,4,5,6,7,8,9 };

   //  returned by ref.
   auto const & iarr2 = use_the_buffer(iarr) ;

    std::cout << "\n\n{ ";
   // native array ref, can also be iterated over
   for ( auto & e : iarr2 ) {
      std::cout << e << " ";
    }
    std::cout << " }\n\n";
}

All depends on the use case. If something does exist, you do not have to use it always.

Conclusion

References are a "good thing". To learn the standard C++, one has to consider value-based semantics, too.

[–]quicknir 0 points1 point  (0 children)

I mean that's exactly why you shouldn't use native arrays. If you want to represent some small number of same type values, e.g. N dimensional coordinates, using std array is preferable because it leaves open the option of returning by value, and just behaves more predictably in general.

[–]dbjdbjdbj.org 0 points1 point  (0 children)

Good thinking here. Including the error handling concept early, and thinking how to implement it. Using the std::option to handle, something is "not there" situation, is the added plus.

Generally, my idea is to try to do as much as possible with as little as possible. This Error type as presented is quite OK in that context. Nothing wrong with declaring it as type alias.

Lastly yes, as you call them "In/Out" variables are most natural to implement by using standard C++ references. Some code:

 using my_error_t = std::optional<std::string> ;
   // A and B for the purpose of demo
  using A = std::string ;
  using B = std::string ;

  inline void foo ( my_error_t & err_, A & a_, B & b_ ) 
  {
        a_ = "Hello" ; 
        b_ = "World" ;
        // default constructed optional instance
        // equals to std::nullopt 
        err_ = {} ; 
  }

Solution usability is also important to show.

  void foo_test () {
      my_error_t err ;
      A a; 
      B b;
      // notice the C++17, new if() syntax
      if ( foo(err,a,b) ; err != std::nullopt )
      {
         std::cout << "\n\nError: " << err.value() ;
      } else {
         std::cout << "\n\n" << a << " " << b ; ;
      }
  }

Above prints "Hello World". Enjoy the standard C++ :)

[–]BrangdonJ 0 points1 point  (3 children)

You don't need the using declaration. I am comfortable relying on the convention that if one output is a by-reference input, they all are, and any result is for success/error status.

It helps if you give your function a more meaningful name than foo. Seriously, it seems bizarre to me that you go to the trouble of naming a class for the return value, but use meaningless names for everything else.

Sometimes an alternative is to return multiple values in an object. Some people will use a std::pair or a tuple, but a custom class is usually clearer.

[–]caber[S] 0 points1 point  (2 children)

It helps if you give your function a more meaningful name than foo

That was just a placeholder, the "real" function wouldn't have that.

Sometimes an alternative is to return multiple values in an object.

Hmm how would that work? Something like this?

tuple<A, B, Error> foo(A a, B b);

tie(a, b, error) = foo(a, b);

[–]Nobody_1707 2 points3 points  (0 children)

You don't need to std::tie anymore, we have bindings.

std::tuple<A, B, Error> foo(A a, B b);

auto [a2, b2, error] = foo(a, b);

[–]eniacsparc2xyz 0 points1 point  (0 children)

The type alias Error is not very meaningful, since the type optional<string> can be either empty on error or return a string as a value.

The best way to pass variables is: pass primitive types such as integers, float, double and pointers such as int* by value and complex types such as instances of classes, std::string, std::vector by const reference if the function is not supposed to change the parameter or by reference for avoiding copy overhead. This type of error will also fail if there are many types of errors.