Advertisement

tic tac toe

Started by October 21, 2019 04:47 PM
61 comments, last by 8Observer8 4 years, 10 months ago

phil67rpg, I told you before about writing if, if, if, if, if... Don't do it (unless you like torturing yourself).

 

Think of a way to use a for loop and an if loop together to simplify this. I usually use two if statements instead of one really long if statement. Like so:


// pseudo code

for(int i=0;i<3;i++) {
	for(int j=0;j<3;j++) {
		if(mouse.x > anchor.x + width*i && mouse.x < anchor.x + width*(i+1)) {
			if(mouse.y > anchor.y + height*j && mouse.y < anchor.y + height*(j+1)) {
				// actions at (i, j)
			}
		}
	}
}

 

26 minutes ago, Euthyphro said:

phil67rpg, I told you before about writing if, if, if, if, if... Don't do it (unless you like torturing yourself).


You are somewhat ignoring your own advice :)

The usual way of eliminating long if statements is to shorten your expressions or put sub-expressions in a variable. Using the && operator is far more readable than nested if statements without an else clause.

Something like this:


bool mouse_this = mouse.x > anchor.x + width*i && mouse.x < anchor.x + width*(i+1);
bool mouse_that = mouse.y > anchor.y + height*j && mouse.y < anchor.y + height*(j+1);

if(mouse_this && mouse_that) {
	...
}

 

Advertisement
1 hour ago, Prototype said:

You are somewhat ignoring your own advice :)

The usual way of eliminating long if statements is to shorten your expressions or put sub-expressions in a variable. Using the && operator is far more readable than nested if statements without an else clause.

Now we're moving from objectively-more-concise to opinion territory. I will remember your point in the future though.

You've removed 1 line to add 2 lines which is fine if it is more readable, but I require descriptive variable names other than this and that. What would you call those bools? withinRangeX and withinRangeY are my best guesses thus far.

 

I thought of a problem with your strategy: both conditions are evaluated first, so there is no opportunity to exit the if statement early if a condition fails to be true, and therefore is at best 50% slower. Obviously, this is not important in a mouse click event.

11 minutes ago, Euthyphro said:

Now we're moving from objectively-more-concise to opinion territory. I will remember your point in the future though.

You've removed 1 line to add 2 lines which is fine if it is more readable, but I require descriptive variable names other than this and that. What would you call those bools? withinRangeX and withinRangeY are my best guesses thus far.

This and that were placeholders of course, descriptive names would obviously further improve readability. 'WithinRange' seems appropriate to fit a boolean expression.

Also note that most of the time compilers will be able to optimize the intermediate variables away, so there is generally no performance penalty. Personally I choose readable code over a handful of CPU cycles anyway.

And yes, sometimes you will lose short-circuit evaluation, so in those cases you'll probably have to find another solution. Like breaking down your expressions even more so they don't contain boolean operators.


bool withinRange = mouse.y > anchor.y + height*j && mouse.y < anchor.y + height*(j+1) ? mouse.x > anchor.x + width*i && mouse.x < anchor.x + width*(i+1) : false;

?

Just now, Euthyphro said:


bool withinRange = mouse.y > anchor.y + height*j && mouse.y < anchor.y + height*(j+1) ? mouse.x > anchor.x + width*i && mouse.x < anchor.x + width*(i+1) : false;

?

Haha, got me :) Still an improvement though because now you can tell what the expression means.

Advertisement
2 hours ago, Euthyphro said:

phil67rpg, I told you before about writing if, if, if, if, if... Don't do it (unless you like torturing yourself).

 

Think of a way to use a for loop and an if loop together to simplify this. I usually use two if statements instead of one really long if statement. Like so:



// pseudo code

for(int i=0;i<3;i++) {
	for(int j=0;j<3;j++) {
		if(mouse.x > anchor.x + width*i && mouse.x < anchor.x + width*(i+1)) {
			if(mouse.y > anchor.y + height*j && mouse.y < anchor.y + height*(j+1)) {
				// actions at (i, j)
			}
		}
	}
}

 

Like you said, we're in the realm of opinion at this point, but I'll also register a vote against the multiple 'if' statements. When I see that, I expect it to have logical significance, and have to parse further to realize it doesn't. (It seems like it could also lead the reader to wonder if there's an error in the code or something has accidentally been omitted.)

In any case, another fairly obvious solution would be to move the conditional work to a function. The code looks more or less like a point-in-axis-aligned-box containment test. Factoring that kind of stuff out can be advantageous anyway, and would leave you with something like this:


for(int i=0;i<3;i++) {
    for(int j=0;j<3;j++) {
        if(point_in_box(mouse, anchor, width, height, i, j)) {
            // actions at (i, j)
        }
    }
}

Or some variation thereof. The code is (arguably) now more concise, readable, and logically clear.

I can't edit my post from 2 hours ago sadly. I want to add an advanced and superfluous tip here to add a break inside the for loop because the mouse can't be within two rectangles at the same time. I do it this way to exit two for loops all the time (I refuse to use the abhorrent goto), but if anyone knows a better way, I could use a tip.


// pseudo code

for(int i=0;i<3;i++) {
    for(int j=0;j<3;j++) {
        if(point_in_box(mouse, anchor, width, height, i, j)) {
            // actions at (i, j)
            i=3; // this will exit the first for loop
            break; // this exits the second for loop
        }
    }
}

This would obviously be more important if you were doing two for loops through hundreds of iterations.

 

Good idea, Zakwayda.

As long as we're making incremental improvements, it's probably worth pointing out that i and j can be computed directly without the need for a loop by using integer division, e.g. (not compiled or tested):


i = (mouse.x - anchor.x) / width;
j = (mouse.y - anchor.y) / height;

If i and j are both in [ 0 3 ), you have your cell, else the mouse is outside the board.

[Edit: Ideally you'd factor the vector subtraction out into a separate function, but I wrote it out in this case for clarity.]

wow there is a lot of conversation about if statements. happy halloween

This topic is closed to new replies.

Advertisement