views:

460

answers:

12

My first question on Stack Overflow... :-)

Someone told me about a C++ style difference in their team. I have my own viewpoint on the subject, but I would be interested by pros and cons coming from everyone.

So... In case you have a class property you want to expose via two getters, one read/write, and the other, readonly (i.e. there is no set method). There are at least two ways of doing it:

class T ;

class MethodA
{
   public :
      const T & get() const ;
      T & get() ;

      // etc.
} ;

class MethodB
{
   public :
      const T & getAsConst() const ;
      T & get() ;

      // etc.
} ;

What would be the pros and the cons of each method?

I am interested more by C++ technical/semantic reasons, but style reasons are welcome, too.

EDIT: Note that MethodB has one major technical drawback (hint: in generic code)

A: 

While it appears your question only addresses one method, I'd be happy to give my input on style. Personally, for style reasons, I prefer the former. Most IDEs will pop up the type signature of functions for you.

William Keller
+1  A: 

I don't understand what the advantage of option B is supposed to be.

Michael Burr
To make it explicit. (I don't see that as an advantage, either, it's merely the *perceived* advantage.)
Tanktalus
+10  A: 

C++ should be perfectly capable to cope with method A in almost all situations. I always use it, and I never had a problem.

Method B is, in my opinion, a case of violation of OnceAndOnlyOnce. And, now you need to go figure out whether you're dealing with const reference to write the code that compiles first time.

I guess this is a stylistic thing - technically they both works, but MethodA makes the compiler to work a bit harder. To me, it's a good thing.

Arkadiy
A: 

I would prefer the first. It looks better in code when two things that essentially do the same thing look the same. Also, it is rare for you to have a non-const object but want to call the const method, so that isn't much of a consern (and in the worst case, you'd only need a const_cast<>).

Leon Timmermans
+4  A: 

Personally, I prefer the first method, because it makes for a more consistent interface. Also, to me getAsConst() sounds just about as silly as getAsInt().

On a different note, you really should think twice before returning a non-const reference or a non-const pointer to a data member of your class. This is an invitation for people to exploit the inner workings of your class, which ideally should be hidden. In other words it breaks encapsulation. I would use a get() const and a set(), and return a non-const reference only if there is no other way, or when it really makes sense, such as to give read/write access to an element of an array or a matrix.

Dima
You're right about encapsulation. In that case, the "encapsulation violation" was the right to do, but this is outside the scope of this discussion. :-)
paercebal
I didn't notice that the getter was returning a reference. You're right the non-const getter should probably return by value.
Michael Burr
If you return by value, then your getter should still be const.
Dima
By 'non-const getter' I meant the getter that returns something that's non-const. It should not return a ref to something inside the object. You're right that regardless the getter should be const (in general).
Michael Burr
A: 

The first allows changes to the variable type (whether it is const or not) without further modification of the code. Of course, this means that there is no notification to the developer that this might have changed from the intended path. So it's really whether you value being able to quickly refactor, or having the extra safety net.

jdmichal
+7  A: 

Well, for one thing, getAsConst must be called when the 'this' pointer is const -- not when you want to receive a const object. So, alongside any other issues, it's subtly misnamed. (You can still call it when 'this' is non-const, but that's neither here nor there.)

Ignoring that, getAsConst earns you nothing, and puts an undue burden on the developer using the interface. Instead of just calling "get" and knowing he's getting what he needs, now he has to ascertain whether or not he's currently using a const variable, and if the new object he's grabbing needs to be const. And later, if both objects become non-const due to some refactoring, he's got to switch out his call.

mos
+1  A: 

Given the style precedent set by the standard library (ie begin() and begin() const to name just one example), it should be obvious that method A is the correct choice. I question the person's sanity that chooses method B.

Greg Rogers
A: 

The second one is something related to Hungarian notation which I personally DON'T like so I will stick with the first method.

I don't like Hungarian notation because it adds redundancy which I usually detest in programming. It is just my opinion.

Iulian Şerbănoiu
A: 

Since you hide the names of the classes, this food for thought on style may or may not apply:

Does it make sense to tell these two objects, MethodA and MethodB, to "get" or "getAsConst"? Would you send "get" or "getAsConst" as messages to either object?

The way I see it, as the sender of the message / invoker of the method, you are the one getting the value; so in response to this "get" message, you are sending some message to MethodA / MethodB, the result of which is the value you need to get.

Example: If the caller of MethodA is, say, a service in SOA, and MethodA is a repository, then inside the service's get_A(), call MethodA.find_A_by_criteria(...).

moffdub
I'm confused... My guess is that you missed the question's point. This question is very C++ specific, and would not be easily transposable in other languages (AFAIK, not in C#, nor in Java, nor in JavaScript, Basic, etc.). It is about const-ness and its use in C++.
paercebal
You're probably right. I suppose from my perspective, the only difference that jumped out was the names of the methods.
moffdub
+1  A: 

So, the first style is generally preferable.

We do use a variation of the second style quite a bit in the codebase I'm currently working on though, because we want a big distinction between const and non-const usage.

In my specific example, we have getTessellation and getMutableTessellation. It's implemented with a copy-on-write pointer. For performance reasons we want the const version to be use wherever possible, so we make the name shorter, and we make it a different name so people don't accidentally cause a copy when they weren't going to write anyway.

tfinniga
A: 

The major technological drawback of MethodB I saw is that when applying generic code to it, we must double the code to handle both the const and the non-const version. For example:

Let's say T is an order-able object (ie, we can compare to objects of type T with operator <), and let's say we want to find the max between two MethodA (resp. two MethodB).

For MethodA, all we need to code is:

template <typename T>
T & getMax(T & p_oLeft, T & p_oRight)
{
   if(p_oLeft.get() > p_oRight.get())
   {
      return p_oLeft ;
   }
   else
   {
      return p_oRight ;
   }
}

This code will work both with const objects and non-const objects of type T:

// Ok
const MethodA oA_C0(), oA_C1() ;
const MethodA & oA_CResult = getMax(oA_C0, oA_C1) ;

// Ok again
MethodA oA_0(), oA_1() ;
MethodA & oA_Result = getMax(oA_0, oA_1) ;

The problem comes when we want to apply this easy code to something following the MethodB convention:

// NOT Ok
const MethodB oB_C0(), oB_C1() ;
const MethodB & oB_CResult = getMax(oB_C0, oB_C1) ; // Won't compile

// Ok
MethodA oB_0(), oB_1() ;
MethodA & oB_Result = getMax(oB_0, oB_1) ;

For the MethodB to work on both const and non-const version, we must both use the already defined getMax, but add to it the following version of getMax:

template <typename T>
const T & getMax(const T & p_oLeft, const T & p_oRight)
{
   if(p_oLeft.getAsConst() > p_oRight.getAsConst())
   {
      return p_oLeft ;
   }
   else
   {
      return p_oRight ;
   }
}

Conclusion, by not trusting the compiler on const-ness use, we burden ourselves with the creation of two generic functions when one should have been enough.

Of course, with enough paranoia, the secondth template function should have been called getMaxAsConst... And thus, the problem would propagate itself through all the code...

:-p

paercebal