lilzz
Posts: 411
Joined: Sat Nov 30, 2013 5:27 pm

Improving the using of const_cast

Fri Aug 22, 2014 8:31 am

Code: Select all

Real StatData::mean(Real trim) const
{
  
   const_cast<StatData&>(*this).items.sort();
}
this is bad because const_cast cast away the const of StatData so that it can use the function items.sort() function.

What's a good way to improve that?

User avatar
jeanleflambeur
Posts: 157
Joined: Mon Jun 16, 2014 6:07 am
Contact: Website

Re: Improving the using of const_cast

Fri Aug 22, 2014 9:00 am

Well first of all what is the mean function supposed to do? I see its parameter is unused.
The ideal way to fix it is by making sure its functionality matches the promise made in its signature. If it promises that it will not alter any member it has to keep that promise - and the const_cast is a good indication that the promise is broken.

lilzz
Posts: 411
Joined: Sat Nov 30, 2013 5:27 pm

Re: Improving the using of const_cast

Fri Aug 22, 2014 3:30 pm

jeanleflambeur wrote:Well first of all what is the mean function supposed to do? I see its parameter is unused.
The ideal way to fix it is by making sure its functionality matches the promise made in its signature. If it promises that it will not alter any member it has to keep that promise - and the const_cast is a good indication that the promise is broken.
Hi,
here's the full code

Code: Select all

/***************************************************************************
  Description : Calculates the trimmed mean of the data.
  Comments    : trim defaults to 0.  Trim = 0.5 now gives the median.
***************************************************************************/
Real StatData::mean(Real trim) const
{
   check_trim(trim);
   if (size() < 1)
      err << "StatData::mean: no data" << fatal_error;

   Real result = 0;
   const_cast<StatData&>(*this).items.sort();
   int low = (int)(size()*trim); // starting at 0
   int high = size() - low;
   if (low == high) {
      low--; high++;
   }
   for(int k = low; k < high; k++) 
      result += items[k];
   ASSERT(2*low < size()); // Make sure we're not dividing by zero.
   return result / (size() - 2*low);
}
If I take away const and it becomes
Real StatData::mean(Real trim) would that help? would the const_cast be alright?

User avatar
AndyD
Posts: 2331
Joined: Sat Jan 21, 2012 8:13 am
Location: Melbourne, Australia
Contact: Website

Re: Improving the using of const_cast

Fri Aug 22, 2014 3:58 pm

lilzz wrote:If I take away const and it becomes Real StatData::mean(Real trim) would that help?
Yes, then there would be no need for the const_cast
lilzz wrote:would the const_cast be alright?
It depends on what you mean by alright! Why did you declare your function as Real StatData::mean(Real trim) const;? As Jean says above, by using the const_cast you have broken that promise, so why make the promise in the first place? Do you need to use the mean method on a const StatData object?

User avatar
jeanleflambeur
Posts: 157
Joined: Mon Jun 16, 2014 6:07 am
Contact: Website

Re: Improving the using of const_cast

Fri Aug 22, 2014 4:19 pm

lilzz wrote: Hi,
here's the full code

Code: Select all

/***************************************************************************
  Description : Calculates the trimmed mean of the data.
  Comments    : trim defaults to 0.  Trim = 0.5 now gives the median.
***************************************************************************/
Real StatData::mean(Real trim) const
{
   check_trim(trim);
   if (size() < 1)
      err << "StatData::mean: no data" << fatal_error;

   Real result = 0;
   const_cast<StatData&>(*this).items.sort();
   int low = (int)(size()*trim); // starting at 0
   int high = size() - low;
   if (low == high) {
      low--; high++;
   }
   for(int k = low; k < high; k++) 
      result += items[k];
   ASSERT(2*low < size()); // Make sure we're not dividing by zero.
   return result / (size() - 2*low);
}
If I take away const and it becomes
Real StatData::mean(Real trim) would that help? would the const_cast be alright?
The mean method is declared as const but it changes the order of the items when called. This confuses the compiler and potentially the user of that code. The const_cast is not the correct way to deal with this.
You have 2 options:
1. Remove the const so the method is allowed to change the items without the cast. Remove the cast also as it's not needed anymore
2. Make a copy of the items vector and sort that one instead. Since the copy is not a member you can change it however you want
Both fix the conflict - first by changing the promise you make and the second one by keeping the initial promise.

There is hidden option number 3. You can declare items as mutable and then the compiler will allow you to change the member in a const method. Mutable members are used to hold internal state that is _not_ visible to the outside in any way - so changing it in const methods should have zero consequences outside. Useful for caching data inside an object, etc.
In your case you're changing the order of elements in the items array - so if the order of elements is not visible from the outside then it's safe to change it even in const methods.

User avatar
Redrobes
Posts: 80
Joined: Mon Dec 26, 2011 9:19 pm
Location: S.W. UK
Contact: Website

Re: Improving the using of const_cast

Fri Aug 22, 2014 10:02 pm

First of all its not the full code since you have used (*this).items. Basically the header declaring the class of stat data has a member called "items". The calling of this function will alter items as a byproduct of the call. Therefore its not const at all. So you should not declare it as such. The compiler is flagging this as you are modifying the class contents from a call for which you declared it would not. That's a no no and the const cast is just a bodge to get you by. Its saying - shut up compiler and just do it anyway ! In a professional sense the issue is your code design - it needs rework to sort it out. But if "get you by" is all you need right now then by all means. Making a habit of this tho will lead to the dark side.

Looking at the code I think you need to take a copy of items and sort that instead. Then items wont change and you can get your mean from it.

You may decide to make a member function for items that is a copy constructor with insertion sort built in then it would be faster to copy items too since you wont then have to subsequently sort them any more.

User avatar
AndyD
Posts: 2331
Joined: Sat Jan 21, 2012 8:13 am
Location: Melbourne, Australia
Contact: Website

Re: Improving the using of const_cast

Sat Aug 23, 2014 12:33 am

The same question was asked on stackoverflow, you might find some of the answers interesting.

You may get more responses to your C++ question by asking them on stackoverflow.

Also, consider buying the latest versions of Scott Meyers books. His blog can be very useful as well, although the topics covered in the blog often assume the reader has a considerable amount of existing knowledge.

Return to “C/C++”