🔒 Locked

"else if" in a for loop

Started by Codiak Jun 17, 2010 at 8:44 AM 8 replies 1.7k views

Original Post

Jun 17, 2010 at 8:44 AM

Here is my code:

if (player->GetXPos()+(AniOffsetX/2)+60 >= Grid[0].Pos.x && player->GetXPos()+(AniOffsetX/2) <= Grid[0].Pos.x+60 && player->GetYPos()+(AniOffsetY/2)-60 <= Grid[0].Pos.y && player->GetYPos()+(AniOffsetY/2) >= Grid[0].Pos.y-60)
{
CollisionSideCheck(0);
}
for (int i = 1; i < GridNum; i++)
{
else if (player->GetXPos()+(AniOffsetX/2)+60 >= Grid.Pos.x && player->GetXPos()+(AniOffsetX/2) <= Grid.Pos.x+60 && player->GetYPos()+(AniOffsetY/2)-60 <= Grid.Pos.y && player->GetYPos()+(AniOffsetY/2) >= Grid.Pos.y-60)
{
CollisionSideCheck(i);
}
}
else
{
player->SetCollision(None);
}

And it compiles this error twice:
error C2181: illegal else without matching if

What I have is an object which needs to check if it's colliding with an array of objects which needs to start with an if statement followed by else if statements. To avoid writing else if statements for every object in the array I thought I would put it in a loop but the compiler doesn't like the fact that the else if statement is in a loop. Anyone know a workaround? Thanks.

Decrius

Jun 17, 2010 at 8:53 AM

if (player->GetXPos()+(AniOffsetX/2)+60 >= Grid[0].Pos.x && player->GetXPos()+(AniOffsetX/2) <= Grid[0].Pos.x+60 && player->GetYPos()+(AniOffsetY/2)-60 <= Grid[0].Pos.y && player->GetYPos()+(AniOffsetY/2) >= Grid[0].Pos.y-60){    CollisionSideCheck(0);}else{    bool found = false;    for (int i = 1; i < GridNum; i++)    {        if (player->GetXPos()+(AniOffsetX/2)+60 >= Grid.Pos.x && player->GetXPos()+(AniOffsetX/2) <= Grid.Pos.x+60 && player->GetYPos()+(AniOffsetY/2)-60 <= Grid.Pos.y && player->GetYPos()+(AniOffsetY/2) >= Grid.Pos.y-60)        {            CollisionSideCheck(i);            found = true;        }    }    if (!found)    {        player->SetCollision(None);    }}

PS: I think you should add a function that takes a Grid and a player and tells you whether they collide. That would make the code much clearer.

Codiak

Jun 17, 2010 at 9:12 AM

Wow Thank you Decrius, works perfectly. Didn't expect actual code :)

Brother Bob

Jun 17, 2010 at 9:24 AM

There's no reason to have ha separate case for i=0 outside the for-loop. Loop from 0 instead. Furthermore, the loop must be broken on first successful else-if check.

int i = 0;for(; i < GridNum; ++i){    if (player->GetXPos()+(AniOffsetX/2)+60 >= Grid.Pos.x && player->GetXPos()+(AniOffsetX/2) <= Grid.Pos.x+60 && player->GetYPos()+(AniOffsetY/2)-60 <= Grid.Pos.y && player->GetYPos()+(AniOffsetY/2) >= Grid.Pos.y-60)    {        CollisionSideCheck(i);        break;    }}if(i == GridNum){    player->SetCollision(None);}

BattleMetalChris

Jun 17, 2010 at 9:50 AM

Forgive me if this is a little noob-ish, but wouldn't the 'break;' just break out of the 'if' block, not the 'for' block?

Brother Bob

Jun 17, 2010 at 10:04 AM

break breaks out of the closest loop construct, which is the for-loop.

BattleMetalChris

Jun 17, 2010 at 10:13 AM

Ah! That will make thing easier for me then.. :-D

I've wanted to use break like that a bunch of times, but avoided it for the reason I gave.

Decrius

Jun 17, 2010 at 11:40 AM

Quote:
Original post by BattleMetalChris
Ah! That will make thing easier for me then.. :-D

I've wanted to use break like that a bunch of times, but avoided it for the reason I gave.

Actually, D has a neater implementation I find, which reduces the need for flags/bools in complex loop constructions. It shortens the code thus. It's also faster by default. Compiler would need to do pretty advanced optimization techniques to remove the bools/flags in C code.

See this example: http://www.digitalmars.com/d/1.0/ctod.html#labeledbreak

/offtopic

iMalc

Jun 17, 2010 at 1:46 PM

Quote:
Original post by Brother Bob
break breaks out of the closest loop construct, which is the for-loop.

Or switch statement.

In fact if you have a for-loop with a switch statement inside that, then inside the switch statement if you use a break, it just breaks out of the switch, but if you use a continue inside the switch statement it know to continue the next for-loop iteration.

void foo(){    for (int i=0; i<3; ++i)    {        switch (i)        {            case 0:                cout << "A";                break;            case 1:                cout << "C";                continue;            case 2:                cout << "D";                return;        }        cout << "B";    }}

[wow]

By the way, that if-statement is screaming out to be made into a function, or at least have some of the repetition taken out. Here is a start:

Point p1 = Point(player->GetXPos() + (AniOffsetX/2),                 player->GetYPos() + (AniOffsetY/2));Point p2 = Grid.Pos;if (p1.x+60 >= p2.x && p1.x <= p2.x+60 &&    p1.y-60 <= p2.y && p1.y >= p2.y-60)

That 60 should also be a named constant.

alvaro

Jun 18, 2010 at 12:12 AM

Quote:
Original post by Decrius
Actually, D has a neater implementation I find, which reduces the need for flags/bools in complex loop constructions. It shortens the code thus. It's also faster by default. Compiler would need to do pretty advanced optimization techniques to remove the bools/flags in C code.

See this example: http://www.digitalmars.com/d/1.0/ctod.html#labeledbreak

/offtopic

I also dislike using bool variables for flow control. I don't have a problem using a well-placed goto statement for this type of purpose. Even better than that is giving this chunk of code a function interface and a decent name, and then substituting the break with a return.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.