If statement doesn't want to do else

[sjaguar13]

                if(x1==x2)
	{
		if(x1==x3||x1==x4)
		{
			setDef();
			return false;
		}
	}
	else if(x1==x3 && x1==x4)
	{
		setDef();
		return false;
	}
	else if(x2==x3 && x2==x4)
	{
		setDef();
		return false;
	}
	else if(y1==y2)
	{
		if(y1==y3||y1==y4)
		{
			setDef();
			return false;
		}
	}
	else if(y1==y3 && y1==y4)
	{
		setDef();
		return false;
	}
	else if(y2==y3 && y2==y4)
	{
		setDef();
		return false;
	}
	else
	{
		m_x[0] = x1;
		m_y[0] = y1;
		m_x[1] = x2;
		m_y[1] = y2;
		m_x[2] = x3;
		m_y[2] = y3;
		m_x[3] = x4;
		m_y[3] = y4;
		return true;
	}
	/*...It only works if I put this right here...*/
	m_x[0] = x1;
	m_y[0] = y1;
	m_x[1] = x2;
	m_y[1] = y2;
	m_x[2] = x3;
	m_y[2] = y3;
	m_x[3] = x4;
	m_y[3] = y4;
	return true;

When I set the m_x and m_y equal to x1 and y1 in the else, it just skips over it. I have to put it at the end or the coordinate pairs come out -8700302093. Why doesn't the else get called?

[jfaust]

For instance, right at the beginning, "if(x1==x2)" could succeed, followed immediately by "if(x1==x3||x1==x4)", which could fail, causing control to jump past the rest of the elses.

Jeff

[Linenoise]

I'm assuming setDef() sets the m_x and m_y values to some defaults - if that's not true, then the answer should be obvious. If it is, then you have a hole in the middle. Look at this section:

else if(x1==x2)
{
if(x1==x3||x1==x4)
{
setDef();
return false;
}
}

if x1 == x2, but x1 != x3 or x4, you do nothing, and leave m_x and m_y unset. You have the same problem in the first y1 case.

As a side note, I would recommend not putting in those returns everywhere. You already will exit the if() block, and it makes following the code flow that much harder. If you added code that always runs after this if() block, there are no less than 9 code paths through the if() block. It would be pretty easy to change things later on but miss one return statement. Things would work most of the time, but occassionally the function would break.

- Todd

[sjaguar13]

Ok, thanks. I fixed it, but what about the returns. If I don't put them there, where can I put them so the right thing is returned?

[Paul McKenzie]

Originally posted by sjaguar13
Ok, thanks. I fixed it, but what about the returns. If I don't put them there, where can I put them so the right thing is returned?

bool foo()
{
    bool retval=false;  
    if ( whatever1 )
    {
        retval = false;
    }
    else if (whatever2)
   {
       retval = true;
   }
   else if (whatever3)
  {
      if (whatever4)
         retval = true;
      else
          retval = false;
  }
//.....blah, blah, blah
//...
//.... at the bottom of your function
return retval;
}

Regards,

Paul McKenzie

[Kheun]

Paul did a good example for "single entry single exit" style. The advantage, is that it is neater and also easier to locate the exit point (especially for debugging). Although this style is quite commonly used in C, I don't always follows the rule. This is because there are situations that certain conditions match and I need to return immediately.

bool foo()
{
    bool result = false;
    if(conditon1)    // Return immediately.
       return result;

    while(condition2)
    {
        // Do something.
        if(condition3)
        {
             // Do other thing.
             result = true;
             break;
        }
    }

    return result;
}