Call nonconst member version from const

Question

Possible Duplicate:
How do I remove code duplication between similar const and non-const member functions?

i have two members

A &B::GetA (int i)
{
    return *(m_C[m_AtoC[i]]);
}

const A &B::GetA (int i) const
{
    return *(m_C[m_AtoC[i]]);
}

for now i just duplicate code, but may be there exists nice way to do it. I certanly dont want to deal with type cast from const to non-const.

EDIT: So i want to call one member frm another to avoid code duplication.

A small `#define` would do the trick, but I guess you don't want to use it either ;-) — Sergey Kalinichenko, Jan 18 '13 at 14:37

Kerrek SB · Accepted Answer · 2013-01-18T14:39:54.433

2

[Note the correction; sorry for getting it the wrong way round initially.]

This is an appropriate situation for using a const_cast, and it allows you to deduplicate code by forwarding the call from the non-const function to the corresponding const function:

A & B::GetA(int index) 
{
    return const_cast<A &>(static_cast<B const *>(this)->GetA(index));
}

It's important to note that this should only be done in one direction: You can implement the non-const method in terms of the constant one, but not the other way round: The constant call to GetA() gets a constant reference to the object in question, but since we have additional information that it's actually OK to mutate the object, we can safely cast away the constness from the result.

There's even a chapter on precisely this technique in Scott Meyer's Effective C++.

edited Jan 18 '13 at 14:39

answered Jan 18 '13 at 14:33

Kerrek SB

428,875
83
813
1,025

please notice, that the OP explicitly writes "I certanly dont want to deal with type cast from const to non-const." – Andy Prowl Jan 18 '13 at 14:34
1

@AndyProwl: So what. A const-cast *is* the appropriate solution for this problem. – Kerrek SB Jan 18 '13 at 14:35
I agree (and I am not the one who downvoted it). but since he says that, it would be appropriate to mention explicitly why it is ok – Andy Prowl Jan 18 '13 at 14:36
@KerrekSB I don't think it is. – melpomene Jan 18 '13 at 14:36
2

@KerrekSB Except that your cast is ill-defined when executed on an instance of `B const`. Do it the other way round, `const`-cast in the non-`const` version. – Konrad Rudolph Jan 18 '13 at 14:36
@KonradRudolph: Yeah, you're right -- I fell into the very trap I was trying to caution against! :-S – Kerrek SB Jan 18 '13 at 14:41
@KonradRudolph I don't think it's ill-defined, as the non-const `GetA()` doesn't do any modifications in itself, and the reference will be cast back to `const&` before being passed out of the function. Calling a non-const function on a const object is not UB, only modifying a const object is. – Angew is no longer proud of SO Jan 18 '13 at 14:43
2

@Konrad const-casting (i.e. playing around with the *types*) is not UB. – R. Martinho Fernandes Jan 18 '13 at 14:43
I don't get this: with this latest solution, aren't you casting away the `const`-ness of the reference returned by the `const` version of `GetA()`? isn't that UB? – Andy Prowl Jan 18 '13 at 14:43
@AndyProwl: No. Only *modifying a constant* object is UB. We're not doing that. – Kerrek SB Jan 18 '13 at 14:44
@R.MartinhoFernandes: Konrad was right, I had it the wrong way round initially. I hope he agrees now :-) – Kerrek SB Jan 18 '13 at 14:45
@AndyProwl: It's only UB if the referenced `A` object is actually `const` in its original declaration (in which case there's no valid code that will return an `A&`). – aschepler Jan 18 '13 at 14:45
@KerrekSB: ok but you're returning to the client a modifiable reference, so it's likely the client will do that – Andy Prowl Jan 18 '13 at 14:45
@AndyProwl: The object in question *is not constant*. – Kerrek SB Jan 18 '13 at 14:45
@KerrekSB: ok, i got it. but then i fail to see the problem with the original version – Andy Prowl Jan 18 '13 at 14:46
1

I looked at the revision history. The original was fine as long as GetA is not mutating by itself (and if it does *gasp* whyyyy). No mutation is ever performed. Only types are harmed, never objects. The function still returned a non-modifiable reference. – R. Martinho Fernandes Jan 18 '13 at 14:46
1

The solution seems to have much *more* code that the original. Is that really an advantage? :-) – Bo Persson Jan 18 '13 at 14:48
1

@BoPersson Hello and welcome to C++. – melpomene Jan 18 '13 at 14:48
@BoPersson Yes, it is, because if you ever need to change the actual getter code, you'll (sooner or later) forget to change it the same in both places. – Angew is no longer proud of SO Jan 18 '13 at 14:49
@BoPersson: Imagine if the const version of the function were really long and complex. `strchr`, I'm looking at you :-) – Kerrek SB Jan 18 '13 at 14:50
I am still curious why was the original version wrong. i think comments by @R.MartinhoFernandes are spot on – Andy Prowl Jan 18 '13 at 14:53
@R.MartinhoFernandes: I think the distinction comes from your parenthetical: The original version had less locally visible safety. With the current version, you *know* that you're not doing anything wrong; with the original version, you had to know the body of the non-constant function. (But indeed, for the OP's situation the two versions work the same.) – Kerrek SB Jan 18 '13 at 14:59
1

@KerrekSB wasn't the exercise about the two having *the same* body? If they don't the question is moot. – R. Martinho Fernandes Jan 18 '13 at 15:01
@R.MartinhoFernandes: Hm, OK, fair enough, but the current version leaves no room for error. Its correctness can be seen locally, without checking the implementation of the other function... it just "feels" better :-S – Kerrek SB Jan 18 '13 at 15:09

score 1 · Answer 2 · answered Jan 18 '13 at 14:36

1

You could do something like:

class B {
public:
    A& GetA (int index) { return GetA_impl(index); }
    const A& GetA (int index) const { return GetA_impl(index); }
private:
    A& GetA_impl (int index) const { return *(m_C[m_AtoC[i]]); }
};

I'm not sure it's really worth the effort in this case, but this can be useful if the potentially duplicated code gets more complicated.

answered Jan 18 '13 at 14:36

aschepler

65,919
8
93
144

I assume, if `m_C` is a member, that you're discarding a const qualifier in your private method ... – Useless Jan 18 '13 at 14:38
@Useless: Depends on the type of `m_C`. But if it's something typical like a `std::vector`, this code does not discard a qualifier. – aschepler Jan 18 '13 at 14:42

score 1 · Answer 3 · answered Jan 18 '13 at 15:05

You can avoid const_cast with a little template metaprogramming.

// This meta function returns a const U if T is const, otherwise returns just U.
template <typename T, typename U>
make_same_const<T, U>
{
    typedef typename std::conditional<
            std::is_const<T>::value,
            typename std::add_const<U>::type,
            U
        >::type type;
};


// Generic version of a function. Constness of return type depends on
// constness of T.
// This is a static member template of B.
template <typename T>
make_same_const<T, A>::type& B::GetA(T& obj, int i)
{
    return *(obj.m_C[obj.m_AtoC[i]]);
}

A&       B::GetA(int i)       { return B::GetA(*this, i); }
A const& B::GetA(int i) const { return B::GetA(*this, i); }

Luchian Grigore · Answer 4 · 2013-01-18T15:10:29.190

0

How about

const A &B::GetA (int index) const
{
    return *(const_cast<B*>(this)->GetA(index));
}

edited Jan 18 '13 at 15:10

answered Jan 18 '13 at 14:31

Luchian Grigore

236,802
53
428
594

not that I don't like it, but the OP writes "I certanly dont want to deal with type cast from const to non-const." – Andy Prowl Jan 18 '13 at 14:33
@AndyProwl I understood this to mean "on the call site" – Angew is no longer proud of SO Jan 18 '13 at 14:37
how is this different from the original ? Did you mean to post something similar to Kerrek SB's answer ? – Sander De Dycker Jan 18 '13 at 14:37
@Angew: hmm, ok maybe it is up to some interpretation – Andy Prowl Jan 18 '13 at 14:38
@SanderDeDycker: this *is* the original answer (notice that this is the first answer that was posted) – Andy Prowl Jan 18 '13 at 14:38
@AndyProwl "Original" as in the code in the question above. Note that this answer casts away constness only to add it back in the return type. The cast is completely pointless here. – melpomene Jan 18 '13 at 14:39
The cast is not pointless. It's what makes it compilable. – R. Martinho Fernandes Jan 18 '13 at 15:08
@melpomene the cast is not pointless (try compiling it without it) – Luchian Grigore Jan 18 '13 at 15:09
@SanderDeDycker the cast is not pointless, but I did mean to call `GetA` instead, yes. :D – Luchian Grigore Jan 18 '13 at 15:10
@R.MartinhoFernandes The cast was pointless in the original version of the code. – melpomene Jan 18 '13 at 15:13

score 0 · Answer 5 · answered Jan 18 '13 at 14:37

IMO this isn't enough code (or complexity) to be worth de-duplicating.

As you can see in both the const_cast-based solutions, the cast expression is actually longer than the original code.

If you have a longer or more complex expression you're really worried about, though, please show it.

score 0 · Answer 6 · answered Jan 18 '13 at 14:48

0

Assuming the bodies of GetA() and GetA() const are identical (which means GetA() doesn't modify *this), you can safely use one const_cast to implement the const version using the non-const one:

const A& B::GetA() const {
  return const_cast<B*>(this)->GetA();
}

The non-const GetA() doesn't modify the object, so calling it on a const B object is not undefined. The non-const reference returned by non-const GetA() is converted to a const& before being returned out of GetA() const, so there's no danger of undefined behaviour there either.

answered Jan 18 '13 at 14:48

Angew is no longer proud of SO

156,801
13
318
412

Would the downvoter explain themselves? – Angew is no longer proud of SO Jan 18 '13 at 14:50
Your answer adds nothing new to the other answers that also ignore the "I certanly dont want to deal with type cast from const to non-const" bit. – melpomene Jan 18 '13 at 14:53
@melpomene It's currently the only answer using `const_cast` in the `const` version (where you don't have to use two) and delegating to the other `GetA()`, not re-implementing. I understood OP's comment to refer to casting constness on the *call site.* But point accepted. – Angew is no longer proud of SO Jan 18 '13 at 14:56

Call nonconst member version from const

6 Answers6

Linked