PDA

View Full Version : if statement doesn't behave correctly



esotery
28th July 2012, 22:40
Hello all,

I came across quite unpleasant problem.
Little intro: I created templated Multiset class, suitable for use in Coloured Petri Nets, nothing fancy. As a private data holder, I'm using the QHash class.

Let's have a look on the header:


template<class T>
class Multiset
{
typedef class QHash<T, uint>::iterator Iterator;
typedef class QHash<T, uint>::const_iterator ConstIterator;
public:
Multiset();
virtual ~Multiset();
...
Multiset<T> operator-(const Multiset<T>& other) const;
Multiset<T> operator*(uint scalar) const;

bool operator<=(const Multiset<T>& other) const;
private:
QHash<T, uint> m_values;
};


I took the liberty to post only the relevant part.

Now the method definitions (methods, which doesn't behave correctly):


template<class T>
Multiset<T> Multiset<T>::operator-(const Multiset<T>& other) const
{
Multiset<T> ret = *this;

ConstIterator itOther = other.m_values.constBegin();

if (!(ret <= other))
{
while (itOther != other.m_values.constEnd())
{
ret.m_values[itOther.key()] -= itOther.value();
++itOther;
}
}

return ret;
}
...
template<class T>
bool Multiset<T>::operator<=(const Multiset<T>& other) const
{
bool ret = false;

ConstIterator itThis;
ConstIterator itOther = other.m_values.constBegin();

while (itOther != other.m_values.constEnd())
{
itThis = m_values.find(itOther.key());

if (itThis == m_values.constEnd()
|| itThis.value() <= itOther.value())
{
ret = true;
break;
}
++itOther;
}

return ret;
}


My problem is, that no matter what the lessOrEqual operator returns, it always jumps inside the:


if (!(ret <= other))
{
while (itOther != other.m_values.constEnd())
{
ret.m_values[itOther.key()] -= itOther.value();
++itOther;
}
}


And it is causing the first series of QCOMPARE to fail in this testcase:


void Multiset_mt::operatorMinus()
{
QFETCH(uint, value);
QFETCH(uint, result);

QCOMPARE(value, result);
}

void Multiset_mt::operatorMinus_data()
{
const uint first_color_size = 2;
const uint second_color_size = 1;
const uint third_color_size = 0;
const uint not_valid_sub_size = 0;

Multiset<Color> ms1, ms2, msRes;
Color c1("black"), c2("red"), c3("blue");

ms1.insert(c1);
ms1.insert(c2);
ms1.insert(c2);
ms2.insert(c1);
ms2.insert(c2);
ms2.insert(c3);

msRes = ms1 - ms2;

QTest::addColumn<uint>("value");
QTest::addColumn<uint>("result");

QTest::newRow("NOT VALID Size == 0") << msRes.size(c1) << not_valid_sub_size;
QTest::newRow("NOT VALID Size == 0") << msRes.size(c2) << not_valid_sub_size;
QTest::newRow("NOT VALID Size == 0") << msRes.size(c3) << not_valid_sub_size;

ms1.insert(c1);
ms1.insert(c1);
ms2.remove(c3);

msRes = ms1 - ms2;

QTest::newRow("Size == 2") << msRes.size(c1) << first_color_size;
QTest::newRow("Size == 1") << msRes.size(c2) << second_color_size;
QTest::newRow("Size == 0") << msRes.size(c3) << third_color_size;
}


If anyone could help me with this, it would be really appreciated. Maybe I missed something. Thank you.

I tried to also attach the whole files.

amleto
28th July 2012, 23:41
Your code doesnt work and you think the if statement is what's wrong? ok...

Have you debugged your operator <= ?

esotery
29th July 2012, 07:22
Hello, thank you for your answer.

Actually my code does work, I've written separate module test also for operator <= and it works correctly. Of course I've debugged my code. As I'm saying, the <= returns true, bool is evaluated as false so it should not jump inside the if statement.

I also tried to create a simple stub method like:


bool Multiset<T>::getTrue() const
{
return true;
}


and replace the bool expression with that method so it looked like:


if (!getTrue())
{
while (itOther != other.m_values.constEnd())
{
ret.m_values[itOther.key()] -= itOther.value();
++itOther;
}
}


but it jumps inside the if anyway.

wysota
29th July 2012, 09:47
Please prepare a minimal compilable example reproducing the problem.

amleto
29th July 2012, 12:21
I can tell you for a fact that your code does not work.

Line 33 of your second code block:

if you have a multiset1 with "a", "b", "b" and multiset2 with "a", "b", "c", "d" then
multiset1 <= multiset2 is true
multiset2 <= multiset1 is true

just because "a" is in both sets.

d_stranz
29th July 2012, 15:39
if (!getTrue())
{
while (itOther != other.m_values.constEnd())
{
ret.m_values[itOther.key()] -= itOther.value();
++itOther;
}
}


And what happens when you eliminate the method call altogether and write if ( false ) as the conditional? If it still fails to execute correctly, then I would strongly suspect that your stack is corrupted by something you did earlier on and that the failing conditional is simply an artifact.

That's why Wysota says, "Provide a minimal compilable example." You might find that in a minimal case things magically start working properly, which should be taken as a strong hint that the problem is not where you are looking.

esotery
29th July 2012, 18:59
amleto: And what is wrong with that? Operator is less OR equal. I think it returns correct result.

d_stranz: hello, yes I have tried to change the code in that way and it seems that when I use value that is known in compile time, it works correctly.

wysota: unfortunately I am away from my work computer and I am leaving for a vacation for a two weeks

amleto
29th July 2012, 21:14
so you're saying that 'less than or equal' is true if both sets have a common element? That's... strange.

to clarify:

{a,b,b,b,b} is less than or equal to {a}?

d_stranz
1st August 2012, 03:46
unfortunately I am away from my work computer and I am leaving for a vacation for a two weeks

Maybe you will be lucky and the code elves will fix your program while you are away.