const correctness gone wrong I suspect...

**matwsij** · 06-26-2011

Hi, I've been having a few headaches trying to get my code to compile. I feel like I've tried every way of changing "const"-ness, but still can't get this to compile with g++ in a cygwin environment.

Would you be so kind to tell me where on earth I am going wrong? Below is the code and the compiler error. The error talks about "qualifiers" which I assume mean there's a "const" missing somewhere, but for the life of me, I can't figure out where.

Code:

#include <iostream>

using namespace std;

class Test
{
   public:
     Test() : val(0) {}
     Test(int v) : val(v) {}

     int getVal() { return val; }

     bool operator<(const Test& rhs) {
       if (rhs.val < val) return false;
       else return true;
     }

   private:
     int val;
};


template <class T>
inline const T& mymax(const T& a, const T& b)
{
   if (a < b) return b;
   else return a;
}

int main(int argc, char* argv)
{
   int a1 = 10, a2 = 20;
   const Test t1(10); 
   const Test t2(20);

   int result = mymax(a1,a2);
   const Test result2 = mymax(t1,t2);

   cout << "Largest value of ints is: " << result << endl;
   cout << "Largest value of Test(ints) is: " << result2.getVal() << endl;
   
}

The compiler generates this error (g++):

Code:

maxTamplate.cpp: In function `const T& mymax(const T&, const T&) [with T = Test]`:
maxTemplate.cpp:37:    instantiated from here
maxTemplate.cpp:26: error: passing `const Test' as `this' argument of `bool Test::operator<(const Test&)' discards qualifiers

I've no idea what this message is trying to tell me

**Elysia** · 06-26-2011

Code:

     int getVal() const { return val; }

     bool operator<(const Test& rhs) const {
       if (rhs.val < val) return false;
       else return true;
     }

The later will actually fix the problem. Remember that any getter--any function that does not change any class member variables--should be const.
The error is essentially trying to say that you are calling a non-const function (operator <) on a const object.

**matwsij** · 06-26-2011

Many thanks Elysia. That's cleared up something I've read and vaguely know about, but have never implemented before. I appreciate
your help.

**laserlight** · 06-26-2011

Incidentally, instead of writing:

Code:

if (rhs.val < val) return false;
else return true;

you can write:

Code:

return rhs.val >= val;

**iMalc** · 06-26-2011

Originally Posted by laserlight

Incidentally, instead of writing:

Code:

if (rhs.val < val) return false;
else return true;

you can write:

Code:

return rhs.val >= val;

Which as it happens means that this classes less-than operator is incorrect, and thus cannot be used to store the item in a std::set for example. The problem is that for two equal items a and b, a<b and b<a would both return true. This is illogical and causes std::set to barf.
I suggest changing it to this:

Code:

       return val < rhs.val;

**kmdv** · 06-26-2011

Originally Posted by matwsij

Code:

template <class T>
inline const T& mymax(const T& a, const T& b)
{
   if (a < b) return b;
   else return a;
}

a and/or b may be temporary objects and (an invalid) reference to an already destroyed object will be returned. Return by value.

**Elysia** · 06-26-2011

I believe it is quite safe. A small experiment seems to confirm this:

Code:

#include <iostream>

class XTest
{
public:
	XTest(int n): m_n(n) { std::cout << "Constructing " << n << "...\n"; }
	~XTest() { std::cout << "Destructing " << m_n << "...\n"; m_n = 0xFFFFFFFF; }
	bool operator < (const XTest& rhs) const { return m_n < rhs.m_n; }
	int m_n;
};

template<typename T>
const T& max(const T& lhs, const T& rhs)
{
	if (lhs < rhs)
		return rhs;
	else
		return lhs;
}

int main()
{
	XTest t1(1), t2(2), t3(3);
	std::cout << "Max of t1 and t2 is: " << max(t1, t2).m_n << "\n";
	std::cout << "Max of t3 and t4 is: " << max(t3, XTest(4)).m_n << "\n";
}

(Yes, I know this is neither good practice nor portable; but then again, this is merely a quick and dirty test with the MSVC compiler; your mileage may vary.)
Output:

Constructing 1...
Constructing 2...
Constructing 3...
Max of t1 and t2 is: 2
Constructing 4...
Max of t3 and t4 is: 4
Destructing 4...
Destructing 3...
Destructing 2...
Destructing 1...

We clearly see that the lifetime of t4 is extended to after the statement.
I cannot be certain, though, as I am unsure exactly what the standard mandates about this.

**whiteflags** · 06-26-2011

The standard library uses the same signature for max as Elysia's code does and the only difference between Elysia's code and the code in question is use of inlining.

**iMalc** · 06-27-2011

See section 12.2 point 5 of the C++0x standard final draft
In short, it is fine.

**kmdv** · 06-27-2011

o_O

So it is correct, good to know.

Thread: const correctness gone wrong I suspect...

Thread Tools

Search Thread

Display

const correctness gone wrong I suspect...

Similar Threads

Program crashes I suspect variable n.

deep const correctness

Suspect my CMOS battery is dying

const correctness and static member variables