Out parameters and pass by reference [closed]

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 6 years ago.

开发者_如何学JAVA Improve this question

I have joined a new group that has coding guidelines that (to me) seem dated.

But just rallying against the machine without valid backup is not going to get me anywhere.

So I am turning to SO to see if we can up with rational reasons for/against (hey I may be wrong in my option so both sides of the argument would be appreciated).

The guideline that is up for argument is:

Tip: Use pointers instead of references for return arguments.

void Func1( CFoo &Return );  // bad  
void Func2( CFoo *pReturn ); // good  

Justification:

When you use a reference, it looks the same as a value. The caller may be surprised that his value has been changed after calling the function. The callee may innocently modify the value without meaning to affect the caller's value. By using a pointer, it is clear to both the caller and callee that the value can be changed. Using references can be particularly misleading in code reviews.

---

When you use a reference, it looks the same as a value.

Only if you really aren't paying attention to what you are doing. Ok, sometimes that happens, but really... no amount of coding standards can correct for people not paying attention or not knowing what they are doing.

The caller may be surprised that his value has been changed after calling the function.

If you are surprised by what happens when you call a function, then the function is poorly documented.

Given a function's name, its parameter list, and perhaps some very brief and descriptive documentation, it should be eminently clear what the function does and what its observable side effects are (including whether any arguments are modified).

The callee may innocently modify the value without meaning to affect the caller's value.

If the function is const correct, then this isn't a problem. If the function isn't const correct, then it should be made const correct, if you can (retroactively making code const correct can be an absolute beating).

This rationale doesn't make much sense, though: when you are actually writing the code for a function, you should be able to see the declarations of the parameters. If the function is so long that you can't, it's time for refactoring.

By using a pointer, it is clear to both the caller and callee that the value can be changed.

This is not entirely correct. A function can take a pointer to const object, in which case the object cannot be changed.

Using references can be particularly misleading in code reviews.

Only if the people doing the code reviews don't know what they are doing.

---

All of that is well and good, but why should pass-by-reference be used instead of pass-by-pointer? The most obvious reason is that a reference cannot be null.

In a function that takes a pointer, you have to check that the pointer is not null before you use it, at least with a debug assertion. During a proper code review you have to analyze more code to be sure that you don't accidentally pass a null pointer to a function that doesn't expect one. I've found that it takes much longer to review functions that take pointer arguments for this very reason; it's so much easier to get it wrong when using pointers.

---

It seems to me that the proper use of const would (mostly) eliminate the need for that tip. The part that still seems useful is when reading caller code, seeing:

Func1(x);

it isn't quite clear what is being done with x (particularly with a nondescript name like Func1). Instead using:

Func2(&x);

with the above convention, indicates to the caller that they should expect x to be modified.

---

If you have not already, buy a copy of Herb Sutter and Andrei Alexandrescu's "C++ Coding Standards: 101 Rules, Guidelines and Best Practices." Read it. Recommend it to your co-workers. It's a good base for a local coding style.

In Rule 25, the authors recommend:

"Prefer passing by reference if the argument is required and the function won't store a pointer to it or otherwise affect its ownership. This states that the argument is required and makes the caller responsible for providing a valid object."

"Argument is required" means NULL is not a valid value.

One of the most frequent causes of defects is accidental de-referencing of null pointers. Using references instead of pointers in these cases can eliminate these at compile-time.

So you have a trade-off -- eliminate a frequent source of errors, or ensure understandability of calling code by means other than the function name. I personally lean toward eliminating risk.

---

While I wouldn't use the tip's advice myself, the justification is valid, which is why languages like C# introduced the out and ref keywords for use at the call site.

The best argument I can come up for against it is this: instead of requiring people to use pointers, you should instead require that people write function names that reflect what the function does. When I call std::swap, I know it's going to change the value of the arguments because the name implies that. On the other hand, if I were to call a function getSize, I wouldn't expect that to modify any arguments.

---

Coding standards are based on habits as much as common sense. Some of your coworkers may rely on years of ingrained assumptions that a parameter not passed by pointer won't change - have pity on them.

The important part of coding standards is not that they're optimal, but that they're adhered to by everybody so that there's some consistency to the body of code.

---

If they really want explicit mention of out parameters at the call site, they should actually require that instead of hacking around it by trying to make pointers mean something they don't. Pointers don't imply modification any more than references do, and it's not uncommon to pass pointers for non-modified objects.

One potential way to express out parameters explicitly:

template<class T>
struct Out {
  explicit Out(T& obj) : base(obj) {}
  T& operator*() { return base; }
  T* operator->() { return &base; }
private:
  T& base;
};
template<class T>
Out<T> out(T& obj) {
  return Out<T>(obj);
}

void f(Out<int> n) {
  ++*n;
}

int main() {
  int n = 3;
  f(out(n));
  cout << n << '\n';
}

And as a temporary measure until they change old code to this, you can make the Out convertible to a pointer and/or reference:

// in class definition
operator T*() { return &base; }
operator T&() { return base; }

// elsewhere
void old(int *p);
void g() {
  int n;
  old(out(n));
}

I went ahead and wrote the various classes required for this, and for in-out parameters, in a way that should degrade nicely. I doubt I'll be using that convention any time soon (in C++, at least), but it'll work for anyone that wants to make call sites explicit.

---

I found there are two schools of though about this:

• (a) use a pointer to show a parameter may be modified
• (b) use a pointer if and only if the parameter may be null.

I agree with your motivation for (a): when reading code, you can't know all declarations, even if a mouseover gives you the declaration of the function. Mousing over hundreds of functions in thousands of lines just takes time.

I certainly see a problem here if you mix in and out parameters:

bool GetNext(int index, Type & result);

A call to this fuinction would look like this:

int index = 3;
Type t; 
if (!GetNext(index, t)) 
  throw "Damn!";

In that example, the call itself is fairly obvious, to potentially modify t. But what about index? Maybe GetNext increments the index, so you always get the next item, without the callee needing to keep caller state?

Which usually raises the reply Then the method should be GetNextAndIncrementIndex, or you should use an iterator anyway. I bet these people never had to debug code written by electrical engineers that still think Numerical Recipes is the Holy Grail of programming.

Howver I still tend to (b): simply because the problem can be avoided for new code being written, and "may be null or not" is usually the more common problem.

---

The justification is logically true.
It may surprise coders that the value has changed (because they thought the value was being passed by value).

But does logically true provide any meaning in this context.
So the value may change. How does this affect the correctness of the code?
Apart from it may print out a different value then an illogical human expects, but the code is doing what it is supposed to be doing and the compiler is enforcing constraints.

---

i recommend:

• pass by reference (do not pass by pointer)
• pass by const reference wherever possible (assuming you've used const correctly throughout your codebase)
• place arguments/parameters which mutate at the beginning of the list
• label the function appropriately
• label the argument appropriately (and create methods/functions with detailed and descriptive names and few arguments)
• document the result
• if multiple arguments/parameters mutate, consider creating a simple class which holds these arguments (even if by reference themselves)
• if they still can't function (sic) without visual and documented cues, create a lightweight template container object for the parameter which mutates, which is then passed to the method or function

---

I would disagree with this guideline. The confusion mentioned in the justification can be easily resolved by making sure the code is const-correct. If you are passing an input parameter to a function by reference, then it should be a const reference. If the reference is not const, that is an indication that it is an output parameter, whose value may be changed by the function.

Furthermore, when you pass a pointer to a function, rather than a reference, that instantly raises a question about whether or not this is a pointer to dynamically allocated memory, and whether or not it should be freed. Using a reference removes the temptation to call delete.

There are times when passing a pointer is appropriate, such as when it actually is a pointer to a dynamically allocated object or array, or when it makes sense for it to be null. Although, you should prefer a smart pointer in such cases. In all other cases a reference is better, IMHO.