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:
- providing two copies of a given function - the const and non-const versions
- or just one version, and then requiring consumers of that code to do their casts as they will?
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:
- do you find this to be an unfortunate by-product of the C++ const-system
- or do you find this to be tantamount to touching the very heights of mount Olympus?
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.
You could use templates.
and use them like
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 ;-)
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.
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:
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.
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 modifypidl
, so you can rewrite it like this:Then clients can call it with a const or non-const
ITEMIDLIST
and it will just work: