Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

In C#, passing a parameter by reference can't be ignored:

// C#
void foo(ref int p_value) ; // passing an int by reference

void bar()
   int i = 0 ;
   foo(ref i) ;

In C++ (and as far as I am concerned, it is a good thing), the passing by reference is only marked in the function prototype, so the code above becomes:

// C++
void foo(int & p_value) ; // passing an int by reference

void bar()
   int i = 0 ;
   foo(i) ;     // at call site, there is no "ref" marker.

The thing is, for a few special functions, to ease code reviewing, we would want to use a C#-like solution to read at call site if a variable is expected to be modified or not.

I devised a solution wrapping the value with a struct, something like:

template<typename T>
struct Ref
   T & m_value ;
   explicit Ref(T & p_value) : m_value(p_value) {}

   operator T & () { return this->m_value ; }
   operator const T & () const { return this->m_value ; }
} ;

template <typename T>   // wrapping T with Ref
Ref<T> byRef(T & p_value)
   return Ref<T>(p_value) ;

template <typename T>   // idempotent-enabling overload (no need for Ref<Ref<T>>)
Ref<T> & byRef(Ref<T> & p_value)
   return p_value ;

I expect this trivial code to be inlined away in optimized code.

This enables me to write the code:

void foo(Ref<int> p_int)
   p_int *= 10 ;

void bar()
   int i = 0 ;
   foo(byRef(i)) ;

Which is exactly what I want:

  • no use of foo(& i) C-style notation, with pointers, needing to handle NULL, etc.
  • in the cases we care, we can see in the code what is passed by non-const reference

Did I miss some unexpected problem/hidden cost that will come back to bite me in a few weeks/months/years?

Could this code be written better?

Will the code be confusing when seen through some un*x debuggers?

P.S.: Of course, Ref and byRef will be properly namespaced. I'm still not sure about making Ref::m_value public or private.


locked by Jamal Aug 4 '14 at 20:59

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

I can't see anything I would do majorly different.

  • I would make the member T & m_value private.
  • I would change the name of the method byRef() to just ref()
    • Note you still have a difference in identifiers.
      The class is Ref while the function is ref()

The only problem I see is that at inside the function, the object is not automatically usable in its native type you need to cast it back before you can use it:

struct X { void print();};

void print(Ref<X> x)
    x.print();  // This fails to compile

    // To use the x object I need to cast it back to an X

    // If we are going to use it multiple times then we could set up
    // a local reference to the object as this will generate an implicit cast
    X&  x1 = x;

Thus getting at methods in the object becomes slightly on the messy side.

I get your point about the method calls. I didn't think about that. . . o.O . . . The good thing is that the aim is to wrap built-ins. . . And of course, you're right about the names: byRef should become ref. – paercebal Nov 17 '12 at 17:41
Note that keeping m_value public (and giving it a short name) would enable direct manipulation of the variable, including the method calls. As the Ref is used only as a simple wrapper, there's no concern about encapsulation, there. Am I wrong? – paercebal Nov 17 '12 at 17:45
@paercebal: It was only a minor thing (personally I would make it private). BUT I have no strong argument for my perspective; in a code review at work if another engineer really wanted it public I would be fine with that as well. Direct manipulation (via member or casting) boils down to the same thing. There is some extra messing around you need to do. I am sure we can take this one step further (if we use some template/macro hijinks). – Loki Astari Nov 17 '12 at 18:36
If the design goal is only to support built-in types then you should make that explicit by makeing sure it does not compile for other types. – Loki Astari Nov 17 '12 at 18:39
makeing sure it does not compile for other types : You're right. On my computer, my own proof-of-concept had additional basic constraints, but I'm unsure about the exact level of conformance/efficiency of all the compilers at my new job (started since two weeks ago... ^_^...), so I'm not sure I'll apply that "patch" until I'm more familiar about those (I already broke the build once for a dumb warning, not sure I want to try it again soon)... ^_^ – paercebal Nov 18 '12 at 12:05

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