CODEWITHSUNDEEP
c++
Mordachai
Mordachai

Reputation: 9662

A recurring const-connundrum

I often find myself having to define two versions of a function in order to have one that is const and one which is non-const (often a getter, but not always). The two vary only by the fact that the input and output of one is const, while the input and output of the other is non-const. The guts of the function - the real work, is IDENTICAL.

Yet, for const-correctness, I need them both. As a simple practical example, take the following:

inline const ITEMIDLIST * GetNextItem(const ITEMIDLIST * pidl)
{
    return pidl ? reinterpret_cast<const ITEMIDLIST *>(reinterpret_cast<const BYTE *>(pidl) + pidl->mkid.cb) : NULL;
}

inline ITEMIDLIST * GetNextItem(ITEMIDLIST * pidl)
{
    return pidl ? reinterpret_cast<ITEMIDLIST *>(reinterpret_cast<BYTE *>(pidl) + pidl->mkid.cb) : NULL;
}

As you can see, they do the same thing. I can choose to define one in terms of the other using yet more casts, which is more appropriate if the guts - the actual work, is less trivial:

inline const ITEMIDLIST * GetNextItem(const ITEMIDLIST * pidl)
{
    return pidl ? reinterpret_cast<const ITEMIDLIST *>(reinterpret_cast<const BYTE *>(pidl) + pidl->mkid.cb) : NULL;
}

inline ITEMIDLIST * GetNextItem(ITEMIDLIST * pidl)
{
    return const_cast<ITEMIDLIST *>(GetNextItem(const_cast<const ITEMIDLIST *>(pidl));
}

So, I find this terribly tedious and redundant. But if I wish to write const-correct code, then I either have to supply both of the above, or I have to litter my "consumer-code" with const-casts to get around the problems of having only defined one or the other.

Is there a better pattern for this? What is the "best" approach to this issue in your opinion:

Or is there a better approach to the issue entirely? Is there work being done on the language itself to mitigate or obviate this issue entirely?

And for bonus points:

EDIT:

If I supply only the first - takes const returns const, then any consumer that needs to modify the returned item, or hand the returned item to another function that will modify it, must cast off the constness.

Similarly, if I supply only the second definition - takes non-const and returns non-const, then a consumer that has a const pidl must cast off the constness in order to use the above function, which honestly, doesn't modify the constness of the item itself.

Maybe more abstraction is desirable:

THING & Foo(THING & it);
const THING & Foo(const THING & it);

I would love to have a construct:

const_neutral THING & Foo(const_neutral THING & it);

I certainly could do something like:

THING & Foo(const THING & it);

But that's always rubbed me the wrong way. I am saying "I don't modify the contents of your THING, but I'm going to get rid of the constness that you entrusted me with silently for you in your code."

Now, a client, which has:

const THING & it = GetAConstThing();
...
ModifyAThing(Foo(it));

That's just wrong. GetAConstThing's contract with the caller is to give it a const reference. The caller is expected NOT TO MODIFY the thing - only use const-operations on it. Yes, the caller can be evil and wrong and cast away that constness of it, but that's just Evil(tm).

The crux of the matter, to me, is that Foo is const-neutral. It doesn't actually modify the thing its given, but its output needs to propagate the constness of its argument.

NOTE: edited a 2nd time for formatting.

Upvotes: 9

Views: 679

Answers (11)

Adam Badura
Adam Badura

Reputation: 5339

During my work I developed a solution similar to what Pavel Minaev proposed. However I use it a bit differently and I think it makes the thing much simpler.

First of all you will need two meta-functions: an identity and const adding. Both can be taken from Boost if you use it (boost::mpl::identity from Boost.MPL and boost::add_const from Boost.TypeTraits). They are however (especially in this limited case) so trivial that they can be defined without referring to Boost.

EDIT: C++0x provides add_const (in type_traits header) meta-function so this solution just became a bit simpler. Visual C++ 2010 provides identity (in utility header) as well.

The definitions are following

template<typename T>
struct identity
{
    typedef T type;
};

and

template<typename T>
struct add_const
{
    typedef const T type;
};

Now having that generally you will provide a single implementation of a member function as a private (or protected if required somehow) static function which takes this as one of the parameters (in case of non-member function this is omitted).

That static function also has a template parameter being the meta-function for dealing with constness. Actual functions will the call this function specifying as the template argument either identity (non-const version) or add_const (const version).

Generally this will look like:

class MyClass
{
public:
    Type1* fun(
        Type2& arg)
    {
        return fun_impl<identity>(this, arg);
    }

    const Type1* fun(
        const Type2& arg) const
    {
        return fun_impl<add_const>(this, arg);
    }

private:
    template<template<typename Type> class Constness>
    static typename Constness<Type1>::type* fun_impl(
        typename Constness<MyClass>::type* p_this,
        typename Constness<Type2>::type& arg)
    {
        // Do the implementation using Constness each time constness
        // of the type differs.
    }
};

Note that this trick does not force you to have implementation in header file. Since fun_impl is private it should not be used outside of MyClass anyway. So you can move its definition to source file (leaving the declaration in the class to have access to class internals) and move fun definitions to source file as well.

This is only a bit more verbose however in case of longer non-trivial functions it pays off.

I think it is natural. After all you just said that you have to repeat the same algorithm (function implementation) for two different types (const one and non-const one). And that is what templates are for. For writing algorithms which work with any type satisfying some basic concepts.

Upvotes: 1

Pavel Minaev
Pavel Minaev

Reputation: 101615

I don't believe it's the deficiency of const-correctness per se, but rather the lack of convenient ability to generalize a method over cv-qualifiers (in the same way we can generalize over types via templates). Hypothetically, imagine if you could write something like:

template<cvqual CV>
inline CV ITEMIDLIST* GetNextItem(CV ITEMIDLIST * pidl)
{
    return pidl ? reinterpret_cast<CV ITEMIDLIST *>(reinterpret_cast<CV BYTE *>(pidl) + pidl->mkid.cb) : NULL;
}

ITEMIDLIST o;
const ITEMIDLIST co;


ITEMIDLIST* po = GetNextItem(&o); // CV is deduced to be nothing
ITEMIDLIST* pco = GetNextItem(&co); // CV is deduced to be "const"

Now you can actually do this kind of thing with template metaprogramming, but this gets messy real quick:

template<class T, class TProto>
struct make_same_cv_as {
    typedef T result;
};

template<class T, class TProto>
struct make_same_cv_as<T, const TProto> {
    typedef const T result;
};

template<class T, class TProto>
struct make_same_cv_as<T, volatile TProto> {
    typedef volatile T result;
};

template<class T, class TProto>
struct make_same_cv_as<T, const volatile TProto> {
    typedef const volatile T result;
};

template<class CV_ITEMIDLIST>
inline CV_ITEMIDLIST* GetNextItem(CV_ITEMIDLIST* pidl)
{
    return pidl ? reinterpret_cast<CV_ITEMIDLIST*>(reinterpret_cast<typename make_same_cv_as<BYTE, CV_ITEMIDLIST>::result*>(pidl) + pidl->mkid.cb) : NULL;
}

The problem with the above is the usual problem with all templates - it'll let you pass object of any random type so long as it has the members with proper names, not just ITEMIDLIST. You can use various "static assert" implementations, of course, but that's also a hack in and of itself.

Alternatively, you can use the templated version to reuse the code inside your .cpp file, and then wrap it into a const/non-const pair and expose that in the header. That way, you pretty much only duplicate function signature.

Upvotes: 3

Jerry Coffin
Jerry Coffin

Reputation: 490328

The real problem here appears to be that you're providing the outside world with (relatively) direct access to the internals of your class. In a few cases (e.g., container classes) that can make sense, but in most it means you're providing low-level access to the internals as dumb data, where you should be looking at the higher-level operations that client code does with that data, and then provide those higher-level operations directly from your class.

Edit: While it's true that in this case, there's apparently no class involved, the basic idea remains the same. I don't think it's shirking the issue either -- I'm simply pointing out that while I agree that it is an issue, it's only that arises only rather infrequently.

I'm not sure low-level code justifies such things either. Most of my code is much lower level than most people ever have much reason to work with, and I still only encounter it rather infrequently.

Edit2: I should also mention that C++ 0x has a new definition of the auto keyword, along with a new keyword (decltype) that make a fair number of things like this considerably easier to handle. I haven't tried to implement this exact function with them, but this general kind of situation is the sort of thing for which they're intended (e.g., automatically figuring out a return type based on passed arguments). That said, they normally do just a bit more than you want, so they might be a bit clumsy (if useful at all) for this exact situation.

Upvotes: 4

Gunther Piez
Gunther Piez

Reputation: 30449

You could use templates.

template<typename T, typename U>
inline T* GetNextItem(T* pidl)
{
    return pidl ? reinterpret_cast<T*>(reinterpret_cast<U*>(pidl) + pidl->mkid.cb) : NULL;
}

and use them like

ITEMDLIST* foo = GetNextItem<ITEMDLIST, BYTE>(bar);
const ITEMDLIST* constfoo = GetNextItem<const ITEMDLIST, const BYTE>(constbar);

or use some typedefs if you get fed up with typing.

If your function doesn't use a second type with the same changing constness, the compiler will deduce automatically which function to use and you can omit the template parameters.

But I think there may be a deeper problem hidden in the structure for ITEMDLIST. Is it possible to derive from ITEMDLIST? Almost forgot my win32 times... bad memories...

Edit: And you can, of course, always abuse the preprocessor. Thats what it's made for. Since you are already on win32, you can completly turn to the dark side, doesn't matter anymore ;-)

Upvotes: 0

justin
justin

Reputation: 104698

You've got a few workarounds now...

Regarding best practices: Provide a const and a non-const versions. This is easiest to maintain and use (IMO). Provide them at the lowest levels so that it may propagate most easily. Don't make the clients cast, you're throwing implementation details, problems, and shortcomings on them. They should be able to use your classes without hacks.

I really don't know of an ideal solution... I think a keyword would ultimately be the easiest (I refuse to use a macro for it). If I need const and non-const versions (which is quite frequent), I just define it twice (as you do), and remember to keep them next to each other at all times.

Upvotes: 2

Laurence Gonsalves
Laurence Gonsalves

Reputation: 143274

From your example, this sounds like a special case of having a pass-through function, where you want the return type to exactly match the parameter's type. One possibility would be to use a template. eg:

template<typename T>  // T should be a (possibly const) ITEMIDLIST *
inline T GetNextItem(T pidl)
{
    return pidl
        ? reinterpret_cast<T>(reinterpret_cast<const BYTE *>(pidl) + pidl->mkid.cb)
        : NULL;
}

Upvotes: 0

UncleBens
UncleBens

Reputation: 41351

IMO this is an unfortunate by-product of the const system, but it doesn't come up that often: only when functions or methods give out pointers/references to something (whether or not they modify something, a function can't hand out rights that it doesn't have or const-correctness would seriously break, so these overloads are unavoidable).

Normally, if these functions are just one short line, I'd just reduplicate them. If the implementation is more complicated, I've used templates to avoid code reduplication:

namespace
{
    //here T is intended to be either [int] or [const int]
    //basically you can also assert at compile-time 
    //whether the type is what it is supposed to be
    template <class T>
    T* do_foo(T* p)
    {
        return p; //suppose this is something more complicated than that
    }
}

int* foo(int* p)
{
    return do_foo(p);
}

const int* foo(const int* p)
{
    return do_foo(p);
}

int main()
{
    int* p = 0;
    const int* q = foo(p);  //non-const version
    foo(q);  //const version
}

Upvotes: 5

Andreas Brinck
Andreas Brinck

Reputation: 52549

I think it's hard to get around, if you look at something like vector in the STL, you have the same thing:

iterator begin() {
    return (iterator(_Myfirst, this));
}
const_iterator begin() const {
    return (iterator(_Myfirst, this));
}

/A.B.

Upvotes: 1

cwick
cwick

Reputation: 26632

You don't need two versions in your case. A non-const thing will implicitly convert to a const thing, but not vice versa. From the name of you function, it looks like GetNextItem will have no reason to modify pidl, so you can rewrite it like this:

inline ITEMIDLIST * GetNextItem(const ITEMIDLIST * pidl);

Then clients can call it with a const or non-const ITEMIDLIST and it will just work:

ITEMIDLIST* item1;
const ITEMIDLIST* item2;

item1 = GetNextItem(item1);
item2 = GetNextItem(item2);

Upvotes: 0

joshperry
joshperry

Reputation: 42257

I would posit that if you need to cast off the const of a variable to use it then your "consumer" code is not const correct. Can you provide a test case or two where you are running into this issue?

Upvotes: 0

Mark Ransom
Mark Ransom

Reputation: 308392

Your functions are taking a pointer to a pidl which is either const or non-const. Either your function will be modifying the parameter or it won't - choose one and be done with it. If the function also modifies your object, make the function non-const. I don't see why you should need duplicate functions in your case.

Upvotes: 2

Related Questions