How (or if) to avoid const_casts

Hi,

as all C++ books I've read so far tell me that const_casts are something really evil, I am trying to avoid them everywhere I can. However I came up with an issue where I don't really know whether avoiding them is worth the extra work.

I am writing a class for a tree which stores elements of class Leaf. A function search exists with several 100 lines of implementation. The interface looks like:

Code:

---

class Leaf;
class LeafAttributes;

class Tree
{
public:
  [...]
  Leaf& search( const LeafAttributes& );
  const Leaf& search( const LeafAttributes& ) const;
}

---

Now of course I don't want to really implement search twice, as the algorithm does not differ for the const and non-const versions. So what I'm doing at the moment is this:

Code:

---

class Leaf;
class LeafAttributes;

class Tree
{
public:
  [...]
  Leaf& search( const LeafAttributes& a )
  { return const_cast<Leaf&> _search( a ); }
  const Leaf& search( const LeafAttributes& ) const
  { return _search( a ); }
private:
  const Leaf& _search( const LeafAttributes& ) const
}

---

So the question I'm asking: Is there a nice, clean way to implent this without using the const_cast?

Thanks,
Torsten

Why don't you just implement the non-const version, make _search return a non-const and make the const version of search call the non-const version ?

Code:

---

class Leaf;
class LeafAttributes;

class Tree
{
public:
  [...]
  Leaf& search( const LeafAttributes& a )
  { return _search( a ); }
  const Leaf& search( const LeafAttributes& ) const
  { return _search( a ); }
private:
  Leaf& _search( const LeafAttributes& ) const
}

---

Or do I miss something ?

Why do you need a const and a non-const version, anyway?

Searches do not usually change the container, so just the "search () const" version should be sufficient. As for the return type, why would you want to return a non-const reference - in fact, why return a reference at all? If the search fails, what are you going to return? The only reason for returning a reference that I can see would be to do something like:

Code:

---

my_tree.search(whatever) = some_other_leaf_node;

---

and, as I said, that's going to run into trouble if the search fails.

Probably, it's better to return a pointer (then you can return 0 if the search fails), and just have the const version of the function:

Code:

---

Leaf* search(const LeafAttribute&) const;

---

Yves M:
I'm trying this out at the moment, but I have the vague feeling that this will just move the const_cast(s) into the _search function. Anyways, it is a better place for it.

Graham:
You are right about pointers. I'm in fact returning them, not references. However the problem that I need both implementations still remains. Below example illustrates why one shouldn't provide a search function like the one you suggest.

Code:

---

void f( const Tree& t ) // this function cannot change t
{
  Leaf * l = t.search( ... ); // works as t.search is const
  l->change(); // t has effectively changed
}

---

Providing a const and a non-const version is also done by the STL functions.

OK. You need it because you can't overload on return type - it's been a long week and my brain's not firing on all cylinders.

If your tree only holds leaf objects, then you might want to think about hiding the leaf entirely and controlling access to it through the tree class itself. Your search function could then return some sort of handle that you give back to the mutating function. That way, the constness of the tree class also controls the mutability of the leaves without the need for duplicating functions. It depends on the ratio of duplicated functions to modifying functions, as well as considerations such as increased encapsulation and lowered coupling.

I did try to implement the soultuion Yves M. suggested:

Code:

---

private:
  Leaf& _search( const LeafAttributes& ) const

---

This unfortunately just lead to the point that I had to use const_cast's inside the implementation. Most notably I have subclass Branch, which is both a Leaf and a Tree (hmm, something wrong with terminology here, but it does make sense) and overloads the implementation of _search. So, somewhere I have then:

Code:

---

class Branch : public Tree, public Leaf
{
[...]
private:
  Leaf& _search( const LeafAttributes& ) const
[...]
}

Leaf& Branch::_search( const LeafAttributes& ) const
{
  if (...)
    return *const_cast<Branch*>(*this)
}

---

This is not really prettyier than the original solution.

So I'm just starting to accept that there is no magical third way. I either can do two separate installations (const and non-const), or I can use const_casts to have one use the other.

Thanks for the comments anyways.

You can use template. However I've never used template for this purpose. I think your 100-line search code needs refactoring.

You can use template. However I've never used template for this purpose. I think your 100-line search code needs refactoring.

Since you are only searching, you should not be modifying your class. Since you are getting an error, that usually means that something deeper in your code is either:

(a) attempting to modify your class or

(b) you are trying to call a member function that does not have a const counterpart

This has happened to me a lot! In either case, you should fix the code underneath, not attempt to place a band-aid on it. If you do not have access to the underlying code, then I suggest that you either write your own const-safe code that does not access the const-unsafe underlying code. If that is not possible, then you're out of luck. I's complain to someone before using const_cast's in that situation though b/c the const_casts can lead to undefined behavior.

In general, only use const_casts if (a) you understand the code and are very, very, VERY certain it is safe to do so and (b) there is no other way around your problem. I have only come across this once -- when needing to change the state of a member mutex to lock access while I am reading some information. Not only did I "prove" to myself that this was safe, but I tested the code and tested it hard before I let it go.

- Kevin

Kevin,
thanks for the comment. My search code is const-safe, if I return a const reference. In other words I implemented both functions below without const_casts:

Code:

---

Leaf& search( const LeafAttributes& a )
const Leaf& search( const LeafAttributes& ) const

---

Implementation of both functions are 95% identical. Differences are such things as

Code:

---

Leaf& Tree::search( const LeafAttributes& a )
{
  [...]
  Leaf& subsearch_result = branch.search( a );
}

const Leaf& Tree::search( const LeafAttributes& ) const
{
  [...]
  const Leaf& subsearch_result = branch.search( a );
}

---

So using const_cast is a way I saw of eliminating duplicate code. My post is in hope of finding a third way to get rid of both evils: const_cast's AND duplicate code.

About my 100-line search needing refactoring: Well, it's not the search function that is really complex, but more the following:

Code:

---

Leaf& searchAfter( const Leaf&, const LeafAttributes& );
const Leaf& searchAfter( const Leaf&, const LeafAttributes& ) const;

---

I have to think about the template idea a bit.

Thanks everybody for showing such interest.

Quote:

---

Originally posted by treuss
My post is in hope of finding a third way to get rid of both evils: const_cast's AND duplicate code.

---

I think this is both a noble and a resonable goal. I believe Yves had the right idea about how to implement the const function. The problem is that you had compiler errors when doing that. Since you have two functions that work -- one without the const constraint and one with the const constraint -- I would attempt to use the code that implements the const-constraint for both cases. If there was some problem with that, I would look at the differences between the two functions, investigate those differences and then refactor. I want to emphasize though that my first attempt would be just to cut-and-paste my code that has the const-constraints into the function that does not require it per-se. If it compiles fine, then you can implement Yves' suggestion and be done with it!

- Kevin

P.S. If you have the time, then refactoring is usually good -- especially for this problem.