132

Don't you hate it when you have

class Foobar {
public:
    Something& getSomething(int index) {
        // big, non-trivial chunk of code...
        return something;
    }

    const Something& getSomething(int index) const {
        // big, non-trivial chunk of code...
        return something;
    }
}

We can't implement either of this methods with the other one, because you can't call the non-const version from the const version (compiler error). A cast will be required to call the const version from the non-const one.

Is there a real elegant solution to this, if not, what is the closest to one?

Keith Pinson
  • 7,162
  • 5
  • 54
  • 97
Michael
  • 2,426
  • 3
  • 23
  • 17
  • 5
    I think you need to indicate what the big chunk of code might be, and least to show why it can't be put in a separate function. – James Hopkin May 13 '09 at 08:23
  • template lybraries explicitly create a "constType" for certain types. That avoid all mentioned problems with other approaches (requires a little bit discipline). – CoffeDeveloper Aug 14 '14 at 09:10
  • 15
    It was very reasonable to re-ask this question. The suggested duplicate is from 2008, hence we have 2 new standards (c++11|14) now which may provide more elegant solutions than the ones provided in the suggested duplicate. Especially since the suggested const_cast is an awful solution! Note that this question asks for **elegant** solution which is not asked for in the suggested duplicate, nor is it given there. – Herbert Nov 11 '14 at 20:51

8 Answers8

128

I recall from one of the Effective C++ books that the way to do it is to implement the non-const version by casting away the const from the other function.

It's not particularly pretty, but it is safe. Since the member function calling it is non-const, the object itself is non-const, and casting away the const is allowed.

class Foo
{
public:
    const int& get() const
    {
        //non-trivial work
        return foo;
    }

    int& get()
    {
        return const_cast<int&>(const_cast<const Foo*>(this)->get());
    }
};
Silicomancer
  • 7,280
  • 6
  • 49
  • 101
CAdaker
  • 12,581
  • 3
  • 27
  • 30
  • 13
    It's safe provided that foo isn't (and never becomes) a const member. If it does, then you ought to re-implement the non-const version of get (or go back to the drawing board and not make foo const after all), but what you actually do is fail to notice the problem because it all compiles OK. – Steve Jessop May 13 '09 at 15:43
  • 6
    "a const member". Or a const return from a call to some other function. Basically the compiler thinks it's returned as const, but "really" it's returned as non-const because of the cast. So it has to really be non-const in the case where "this" is non-const, and the compiler can't help you enforce that. – Steve Jessop May 13 '09 at 15:57
  • 1
    Of course, the normal rules for const_cast still apply: if foo was defined as const it's not legal to cast it away. This is just for the case where both methods are valid, but we don't want to repeat code. – CAdaker May 13 '09 at 16:03
  • 1
    Sure, but hand-validating the const-correctness might be harder than just c'n'p'ing the code and keeping it in sync with changes. Or it might be easier. So it's important to be aware of the risk. – Steve Jessop May 13 '09 at 16:06
  • But the problem of const-correctness doesn't disappear anyway. If you duplicate the code from the const method you still have a non-const member function returning a non-const reference. This would be exactly the same thing, and casting away the const will be perfectly valid if the copy-and-paste method would be valid. – CAdaker May 13 '09 at 16:17
  • 4
    Right, but if you c'n'p, the compiler will tell you whether the code is valid. If you cast, you have to work it out for yourself. I'm not saying this is impossible, or that it's necessarily a bad idea, but C++ programmers do tend to lean on the compiler, especially for const-correctness and other forms of type safety. That is what it's there for. – Steve Jessop May 13 '09 at 16:29
  • Ok, that is a point. But the most common case is probably that we have a lot of validation and error checking code, which should be easy to check. – CAdaker May 13 '09 at 16:42
  • 24
    `static_cast(this)` should be `const_cast(this)`. – ildjarn Aug 10 '11 at 21:13
  • 2
    @ildjarn: should it ? you need `const_cast` when you want to remove `const` or `volatile` qualifiers, but if you want to add them, `static_cast` is enough. – Raphaël Saint-Pierre Aug 10 '11 at 22:36
  • 3
    @Jeandard : `const` can be added implicitly without a cast, so `static_cast` is doing nothing, as would be `const_cast`. I.e., there is no difference between the two in this scenario whatsoever, because neither is actually doing anything. However for readability's sake, the cast that reflects the type change actually taking place should be used. – ildjarn Aug 10 '11 at 22:55
  • @ildjarn: in contexts where `const` can be added implicitely, it is, but here, we actually need it so as to force the call to the `const` getter (instead of recursing into the non-`const` one). As for `const_cast` vs. `static_cast`, I guess it's a matter of personal preference; to me, `const_cast` feels more dangerous and intended towards `const`-removal. – Raphaël Saint-Pierre Aug 10 '11 at 23:16
  • 1
    @Jeandard : I understand the reason for _a_ cast here; I'm just saying that because only const-ness is being affected, it makes more sense (readability-wise) for the cast being used here to be `const_cast`. Agreed, though -- personal preference, doesn't ultimately matter one way or another. – ildjarn Aug 10 '11 at 23:20
  • 10
    From the answer text: **Since the member function calling it is non-const, the object itself is non-const, and casting away the const is allowed.** After reading this statement many times, I _finally_ understand why this approach is safe and correct. – Boinst Aug 14 '12 at 00:43
  • Not really seeing how replacing a 1-liner with a 1-liner makes much sense, particularly replacing it with a 5x longer 1-liner. I suppose this is just an issue we face with C++. – doug65536 Jan 23 '13 at 23:24
  • 4
    The idea is of course that the "//non-trivial work" is significantly longer than the one-liner in the non-const method. Otherwise it is indeed pointless. – CAdaker Feb 05 '13 at 21:19
  • 1
    Nice reminder of the practical use of `const_cast` in Effective C++. Beware that casting away const only disguises it's access modifier, not the instance entirely. So it's still const after the `const_cast` and undefined behavior you will see if you try to do non-const stuff. – David Lee May 17 '13 at 14:27
  • @doug65536 For a practical example of what CAdaker said, imagine that the pair of functions in question are the `operator(r, c)` for a matrix - which (as it should) checks that the matrix has been initialised/sized, checks that `r` is within the number of rows, checks that `c` is within the number of columns, etc. That quickly gets tedious and error-prone to duplicate. – underscore_d Sep 20 '17 at 09:52
  • 1
    @DavidLee Late but still: If the object actually is non-const, then casting away the ("artificially" added) const and modifying the object is not undefined behavior. It is an utterly necessary and standard language feature. It is only UB if the object was actually *defined* (nor declared) const -- because it may be in RO mem, like in actual ROM for embedded systems. – Peter - Reinstate Monica Jun 29 '18 at 10:41
  • @PeterA.Schneider u r right. ur comment along with this post epitomizes why languages like Java, C# thrown out the idea of bit-wise constness and sticking with logical constness. But I still can't say bit-wise constness is bad as I've seen many cases where it came handy. – David Lee Jun 30 '18 at 12:31
29

How about:

template<typename IN, typename OUT>
OUT BigChunk(IN self, int index) {
    // big, non-trivial chunk of code...
    return something;
}

struct FooBar {
    Something &getSomething(int index) {
        return BigChunk<FooBar*, Something&>(this,index);
    }

    const Something &getSomething(int index) const {
        return BigChunk<const FooBar*, const Something&>(this,index);
    }
};

Obviously you'll still have object code duplication, but no source code duplication. Unlike the const_cast approach, the compiler will check your const-correctness for both versions of the method.

You probably need to declare the two interesting instantiations of BigChunk as friends of the class. This is a good use of friend, since the friend functions are hidden away close to the friendee, so there is no risk of unconstrained coupling (ooh-er!). But I will not attempt the syntax for doing so right now. Feel free to add.

Chances are that BigChunk needs to deference self, in which case the above order of definition isn't going to work very well, and some forward declarations will be needed to sort it out.

Also, in order to avoid some numpty finding BigChunk in the header and deciding to instantiate and call it even though it's morally private, you can move the whole lot into the cpp file for FooBar. In an anonymous namespace. With internal linkage. And a sign saying "beware of the leopard".

Steve Jessop
  • 257,525
  • 32
  • 431
  • 672
  • Can you call `BigChunk` from a function declared as `const`? I don't think you can, because `BigChunk` is not declared as `const`. This is one major limitation of C++ templates. – HelloGoodbye Mar 28 '13 at 07:19
  • @HelloGoodbye: `BigChunk` is not a member function. In the `const` overload of `getSomething`, I instantiate `BigChunk` with `IN` as `const FooBar*`, and so of course it's fine to pass `this` as the first parameter. – Steve Jessop Mar 28 '13 at 12:09
  • Ah, I didn't notice it wasn't! Well, then this solution is actually quite nice. – HelloGoodbye Apr 05 '13 at 11:03
  • 1
    I guess the only problem is that you have to send all private member variables you want to use in the function as arguments to the function, unless you have getter/setter methods for those variables. And then it might be a problem that you have to duplicate the argument types instead. You may even have to provide the template with the types of those arguments since they may have to be given in const/non-const versions. If you would accidentally use the incorrect type, you would probably end up with an implicit cast to another data type that you don't want to use, without noticing it. – HelloGoodbye Apr 05 '13 at 11:09
  • 1
    On the other hand, you could declare the function as a const member function, still providing it with the `this` pointer, and you would automatically be able to access all private members through the `this` pointer. However, then the function would become another symbol that would be visible in all translation units that include the header file, and it would also have to be handled by the linker, which would in turn increase the linking time a little bit. I guess there is always some catch, huh? :) – HelloGoodbye Apr 05 '13 at 11:19
  • @HelloGoodbye this is too far fetched thinking unless you are in ultra limited environment. coding is stress on the build chain, should we care because the build chain will go on strike or something ? But I still agree that it's positive to be aware of costs, any cost. – v.oddou Mar 04 '16 at 05:28
8

I would cast the const to the non-const (second option).

Matthew Flaschen
  • 255,933
  • 45
  • 489
  • 528
4

Try to eliminate the getters by refactoring your code. Use friend functions or classes if only a very small number of other things needs the Something.

In general, Getters and Setters break encapsulation because the data is exposed to the world. Using friend only exposes data to a select few, so gives better encapsulation.

Of course, this is not always possible so you may be stuck with the getters. At the very least, most or all of the "non-trivial chunk of code" should be in one or more private functions, called by both getters.

markh44
  • 5,344
  • 5
  • 25
  • 33
  • 12
    What?! Getters and setters break encapsulation? And friends don't? What if the class inner structure (private part) changes, the variables are renamed or eliminated? do you rewrite all friends? -1 from me... – Paulius May 13 '09 at 08:00
  • 6
    Yes, friends are really too unused. Lots of C++ devs see them as breaking encapsulation, when they instead (by words from Herb Sutter) actually increase encapsulation. – Johann Gerell May 13 '09 at 08:02
  • 6
    Paulius: Imagine you have 2 classes that use each others data. What is the best way to allow access to that data? public getters and setters or making them friends? With public getters and setters, any piece of code could use those getters and setters. With friends, only the friends can access the data. The scope of who can see and/or modify the data is hugely reduced, so yes, friend can give better encapsulation than getters and setters. I really don't know why people panic when friend is mentioned. friend is good. – markh44 May 13 '09 at 08:21
  • 7
    friend is good when the classes are actually good mates. friend is terrible if the classes barely know one another, or if another class comes along that wants to access the data, so you just add another friend. C++ != Facebook, and classes shouldn't respond to friend requests. I'd say if the classes weren't designed together, don't use friend. Once that's out of the way, yes, friend is good :-) – Steve Jessop May 13 '09 at 15:40
  • 2
    Of course I agree and should have made clear in my answer that the classes involved in friendship must be very strongly related. It's not something that should be used for convenience (neither should getters and setters for that matter). – markh44 May 13 '09 at 16:22
  • Indeed, if you're implementing a non-const getter based on an integer index, then what that says to me is operator[] and/or an iterator. Admit that you're a container, basically. But they still have the same const/non-const code duplication issue. – Steve Jessop May 13 '09 at 16:34
  • The only way I could agree to this, is if you make a private getters and setters, and then let friends use those... Accessing private data members of a class can not increase encapsulation in any possible way... Not in my world... – Paulius May 14 '09 at 08:37
  • This explains it better than I could: http://www.parashift.com/c++-faq-lite/friends.html#faq-14.2 – markh44 May 14 '09 at 16:39
  • I agree. I use `friend` a lot when building library classes that will be exposed to other devs. For File manipulation library, you don't want to expose the private FileSystem object with get/set to other devs but you would want to toss it to other helper classes in the same namespace. – David Lee May 17 '13 at 13:31
  • Java thought of `friend` as a nice idea and adopted it in it's own way by introducing `package private` access modifier. In Java, variable with no access modifier will be package private and can be accessed by any class in the same pakcage (namespace), which means it's private to the namespace it belongs. Java heavily uses it to share data between public class and private class in the same package (namespace). java.lang.Character class uses it to access namespace private [Unicode planes and code points](http://en.wikipedia.org/wiki/Unicode_plane) in private classes that inherits CharacterData – David Lee May 17 '13 at 14:00
  • I use friends only when I know the other class. In most cases that is rarely true. Another issue is that with multithreaded code you will have to actually obtain the lock from the other class, and that is hard to maintain. Although it is an interesting approach in single threaded code, I doubt that it will bring too much value in other cases. Although, it is true that it increases encapsulation in aggregate form when taking into account all the befriended classes. Nonetheless, better go with getter/setters as a general rule. – g24l Nov 03 '15 at 09:24
  • I was interested that this was my most downvoted answer. 7-8 years later I think what I wrote is valid. The notion of using friend should not be dismissed out of hand, it is sometimes the best approach. Try writing a serialising free function without friend; you'd have to expose all of your data to everybody! Sure, it's rare that friend is the best approach but sometimes it is. – markh44 Jan 26 '21 at 12:26
3

Why not just pull the common code out into a separate, private function, and then have the other two call that?

  • 5
    The problem is that if the private function is non-const, then the const version can't call it. Conversely, if the private function is const, then it cannot return `Something&` (assuming that the returned value is a member of `Foo`). Therefore, it is impossible for both `Something& getSomething(int index)` and `const Something& getSomething(int index) const` to call the same private method. – sasquires Oct 05 '18 at 17:05
2

The const reference to the object makes sense (you're putting a restriction on read-only access to that object), but if you need to allow a non-const reference, you might as well make the member public.

I believe this is a la Scott Meyers (Efficient C++).

swongu
  • 2,161
  • 1
  • 17
  • 24
  • 1
    Public members? Not very OO-ish... – Michael May 13 '09 at 07:59
  • I might retract this, because I didn't read the part about having "non-trivial code", making those functions not getters directly. – swongu May 13 '09 at 08:01
  • 1
    Actually, when you think of it. If not misused, public members are the simplest and most straight-forward answer to this problem. If anyone can manipulate them by using the proper getter/setter, then in effect it is a public member. Maybe making it public is even better code because it tells the client, "I'm a public member, do anything you want to me". It is much more straight-forward than implementing two types of getters. – Michael May 13 '09 at 10:24
  • 1
    "you might as well make the member public". No, because there's a "big, non-trivial chunk of code" in the getter. This getter isn't just exposing a member, it's doing significant work. Whether it's a good idea to then expose the result as non-const is of course another question... – Steve Jessop May 13 '09 at 15:59
1

The concept of 'const' is there for a reason. To me it establishes a very important contract based on which further instructions of a program are written. But you can do something on the following lines :-

  1. make your member 'mutable'
  2. make the 'getters' const
  3. return non-const reference

With this, one can use a const reference on the LHS if you need to maintain the const functionality where you are using the getter along with the non-const usage(dangerous). But the onus is now on the programmer to maintain class invariants.

As has been said in SO before, casting away constness of an originally defined const object and using it is an U.B. So i would not use casts. Also making a non-const object const and then again casting away constness would not look too good.

Another coding guideline that I have seen used in some teams is:-

  • If a member variable needs to be modified outside the class, always return a pointer to it via a non-const member function.
  • No member functions can return non-const references. Only const references are allowed form const member functions.

This allows some consistency in the overall codebase and the caller can clearly see which calls can modify the member variable.

Abhay
  • 6,743
  • 3
  • 34
  • 47
-2

I dare suggest using the preprocessor:

#define ConstFunc(type_and_name, params, body) \
    const type_and_name params const body \
    type_and_name params body

class Something
{
};

class Foobar {
private:
    Something something;

public:
    #define getSomethingParams \
    ( \
        int index \
    )

    #define getSomethingBody \
    { \
        return something; \
    }

    ConstFunc(Something & getSomething, getSomethingParams, getSomethingBody)
};
Matthew
  • 29
  • 1