Page 1 of 1

Suspect comparision(?) in TRoom.cpp

Posted: Tue Jun 11, 2013 10:38 pm
by SlySven
I note on line 348 of the current code there are multiple comparisons which throw up multiple GCC warnings: "warning: suggest parentheses around comparison in operand of ‘==’" what are these tests supposed to be establishing - is it equality of all four variables because if I recall C/C++ expression evaluation correctly non-equal values such as min_x = -5, min_y = -4, max_x=0, max_y=1 will still cause the overall expression to be evaluated as true?

Code: Select all

void TRoom::calcRoomDimensions()
{
    QMapIterator<QString, QList<QPointF> > it(customLines);
    while( it.hasNext() )
    {
        it.next();
        const QString & _e = it.key();
        const QList<QPointF> & _pL= it.value();
        if( _pL.size() < 1 ) continue;
        if( min_x == min_y == max_x == max_y == 1 )
        {
            min_x = _pL[0].x();
            max_x = min_x;
            min_y = _pL[0].y();
            max_y = min_y;
        }
        for( int i=0; i<_pL.size(); i++ )
        {
            qreal _x = _pL[i].x();
            qreal _y = _pL[i].y();
            if( min_x > _x )
                min_x = _x;
            if( max_x < _x )
                max_x = _x;
            if( min_y > _y )
                min_y = _y;
            if( max_y < _y )
                max_y = _y;
        }
        qDebug()<<"custom lines span: x("<<min_x<<"/"<<max_x<<") y("<<min_y<<"/"<<max_y<<")";
    }
}
That being the case wouldn't it be better to use:

Code: Select all

        if( min_x == max_y && min_y == max_y && min_x == min_y )
?

Re: Suspect comparision(?) in TRoom.cpp

Posted: Wed Jun 12, 2013 9:49 am
by Heiko
SlySven wrote: wouldn't it be better to use:

Code: Select all

        if( min_x == max_y && min_y == max_y && min_x == min_y )
?
No, that's a very different meaning. It's good that you bring up custom lines to the agenda because custom lines definitely needs some love. I put this on my todo list.

Re: Suspect comparision(?) in TRoom.cpp

Posted: Wed Jun 12, 2013 2:23 pm
by SlySven
OK so that bit of code needs some lovin' but what IS it testing for? I was thinking some sort detection that the four variables needed initialisation - and its always good that variables get initialised as I heard that all sorts of weirdness results when uninitialised ones get used... :?

Re: Suspect comparision(?) in TRoom.cpp

Posted: Sun Jun 16, 2013 7:02 am
by Vadi
I think you're looking at old code, this was removed in the last commit (https://sourceforge.net/p/mudlet/code/c ... fb941eade5) and isn't there anymore: https://sourceforge.net/p/mudlet/code/c ... m.cpp#l281

Re: Suspect comparision(?) in TRoom.cpp

Posted: Sun Jun 16, 2013 7:42 am
by Heiko
The entire custom lines feature has never been completed and still needs a lot of work. It's on my todo list for the next release.