Sign in to follow this  
coderWalker

Opposite of If statement

Recommended Posts

Right now I am using this:
[code]if (
world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true &&
world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1
)
{

}
else
{
code
}[/code]

I am trying to clean up this code along with 14 other files. However I can't afford to have the wrong answer.
So what would the if statement need to change to so code can be in the standard if and not the else?

It wouldn't be this, because the && complicates it:
[code]
if (
world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==false &&
world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)!=-1
)
[/code]

Share this post


Link to post
Share on other sites
That's some horrid looking code IMHO

You could do this though

[code]bool test = world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true &&
world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1;

if (!test)
{
...
}

[/code]

Share this post


Link to post
Share on other sites
To explain with a readable example (since you need to do 14 other files).

If you have:

[code]if (a && B) {
} else {
// do something
}[/code]

You can rewrite it as:

[code]if (!a || !B) {
// do something
}[/code]

If your statements inside if () are complex, or you simply can't be bothered with going through it, gazliddon's suggestion is a good alternative. It makes your code a little bit more reabable I suppose. You could also just negate the if statement like this:

[code]// before
if (a && b && c && d && e) {
} else {
// do something
}

// after
if (! (a && b && c && d && e) ) {
// do something
}[/code]

Share this post


Link to post
Share on other sites
[quote name='coderWalker' timestamp='1301520157' post='4792351']
Right now I am using this:
[code]if (
world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true &&
world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1
)
{

}
else
{
code
}[/code]
[/quote]



Easier to read = easier to refactor.

[code]bool blockExists = world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true;
bool blockAmountIsInvalid = world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1;

if (blockExists && blockAmountIsInvalid)
{

}
else
{
code
}[/code]

Now you can easily get rid of the else ...


[code]bool blockExists = world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true;
bool blockAmountIsInvalid= world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1;

if (!blockExists || !blockAmountIsInvalid)
{
code
}[/code]

Although without seeing more of the code, that condition doesn't make much sense to me :P


edit: holy flood of answers, Batman

Share this post


Link to post
Share on other sites
[quote name='teutonicus' timestamp='1301521706' post='4792359']
Easier to read = easier to refactor.

[code]bool blockExists = world0->blockExists(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==true;
bool blockAmountIsInvalid = world0->blockGetAmount(x-1,y,z,world0->xOffset-location.x,world0->yOffset-location.y,world0->zOffset-location.z)==-1;

if (blockExists && blockAmountIsInvalid)
{

}
else
{
code
}[/code]

[/quote]


I think this one ^^ is the better. You can even make it more readable by adding more variables:

[code]

int xof_param = world0->xOffset-location.x;
int yof_param = world0->yOffset-location.y;
int zof_param = world0->zOffset-location.z;

bool blockExists = world0->blockExists (x-1, y, z, xof_param, yof_param, zof_param);
bool blockAmountIsInvalid = world0->blockGetAmount(x-1, y, z, xof_param, yof_param, zof_param)==-1;

if (blockExists && blockAmountIsInvalid)
{

}
[/code]

Share this post


Link to post
Share on other sites
Thanks everyone! I think the best thing to do is just negate the statement.
I never even thought of that solution :P

Yea the If statements are pretty bad.
I really need to make them more readable somehow.

I Always seem to over complicate things.

Share this post


Link to post
Share on other sites
[quote name='coderWalker' timestamp='1301522881' post='4792361']
Thanks everyone! I think the best thing to do is just negate the statement.
I never even thought of that solution :P
[/quote]
No, that's the easy way out. The best thing to do it to understand boolean algebra and design the logic to properly describe what you want it to do.

[quote name='coderWalker' timestamp='1301522881' post='4792361']
Yea the If statements are pretty bad.
I really need to make them more readable somehow.

I Always seem to over complicate things.
[/quote]
In my opinion, what makes your if-statement ugly and difficult to read is the way your function calls look like. Many calculations and multiple parameters that perhaps should be collected in structures. For example, not only should you factor out the calculations like owl demonstrated, but is there a reason you're not sticking your (what appears to be) vectors in a vector structure to collect three linked parameters into one?

If you do, then your two functions only take two parameters, and one can be calculated outside the function calls. Then you don't even have to calculate the individual boolean results like owl did. What you gain by that is short circuiting, in that the second function call doesn't have to be evaluated if the first one completely determines the outcome of the entire expression. And the if-statement doesn't become messy anyway, since the function calls are clean and compact looking.

Share this post


Link to post
Share on other sites
[quote name='Brother Bob' timestamp='1301526018' post='4792376']
[quote name='coderWalker' timestamp='1301522881' post='4792361']
Thanks everyone! I think the best thing to do is just negate the statement.
I never even thought of that solution :P
[/quote]
No, that's the easy way out. The best thing to do it to understand boolean algebra and design the logic to properly describe what you want it to do.

[quote name='coderWalker' timestamp='1301522881' post='4792361']
Yea the If statements are pretty bad.
I really need to make them more readable somehow.

I Always seem to over complicate things.
[/quote]
In my opinion, what makes your if-statement ugly and difficult to read is the way your function calls look like. Many calculations and multiple parameters that perhaps should be collected in structures. For example, not only should you factor out the calculations like owl demonstrated, but is there a reason you're not sticking your (what appears to be) vectors in a vector structure to collect three linked parameters into one?

If you do, then your two functions only take two parameters, and one can be calculated outside the function calls. Then you don't even have to calculate the individual boolean results like owl did. What you gain by that is short circuiting, in that the second function call doesn't have to be evaluated if the first one completely determines the outcome of the entire expression. And the if-statement doesn't become messy anyway, since the function calls are clean and compact looking.
[/quote]

Me and Owl must of posted at the exact same time. I didn't even see it, but now I do. I think Owl's is best also
combining it like he did also saves some computing time. Since it just reads from memory and doesn't have to calculate.

Share this post


Link to post
Share on other sites
Hidden
[quote name='coderWalker' timestamp='1301527894' post='4792383']
Me and Owl must of posted at the exact same time. I didn't even see it, but now I do. I think Owl's is best also
combining it like he did also saves some computing time. Since it just reads from memory and doesn't have to calculate.
[/quote]

What I posted is exactly what teutonicus proposed but I just made the calculations outside the function call for readability's sake. I don't it's gonna be faster and if it is it'd be a despicable improvement, but I've learned to be willing to trade a few cycles for readability and code maintainability

You might want to explore what Brother Bob proposed: Implementing algebraic vectors into your code:

[font="'Courier New"][code][/font]
[font="'Courier New"]class vector3[/font]
[font="'Courier New"]{[/font]
[font="'Courier New"] public: // Make it right[/font]
[font="'Courier New"] int x;[/font]
[font="'Courier New"] int y;[/font]
[font="'Courier New"] int z;[/font]
[font="'Courier New"]};[/font]
[font="'Courier New"]
[/font]
[font="'Courier New"]// make a method in class world that combines [/font]blockExists and blockGetAmount
[font="'Courier New"]// the offset seem to be inside world so you don't even need to pass it as an argument.[/font]
[font="'Courier New"]World::BlockValidExists(vector3& v_pos, vector3& v_xof)[/font]
[font="'Courier New"]{[/font]
[font="'Courier New"] return ([/font][font="'Courier New"]blockExists ([/font]pos_vector, xof_vector) && (blockGetAmount(pos_vector, xof_vector)!=-1));
[font="'Courier New"]}[/font]
[font="'Courier New"]
[/font]
[font="'Courier New"]// and then ...[/font]
[font="'Courier New"]
[/font]

[font="'Courier New"]vector3 xof_vector (world0->xOffset-location.x, [/font]world0->yOffset-location.y, world0->zOffset-location.z);
[font="'Courier New"]vector3 pos_vector ([/font]x-1, y, z)


if (world0->BlockValidExists(pos_vector, xof_vector))
[font="'Courier New"]{[/font]
[font="'Courier New"]
[/font]
[font="'Courier New"][/code][/font]

Share this post


Link to post
[quote name='coderWalker' timestamp='1301527894' post='4792383']
Me and Owl must of posted at the exact same time. I didn't even see it, but now I do. I think Owl's is best also
combining it like he did also saves some computing time. Since it just reads from memory and doesn't have to calculate.
[/quote]

What I posted is exactly what teutonicus proposed but I just made the calculations outside the function call for readability's sake. I don't think it's gonna be faster and if it is it'd be a despicable improvement, but I've learned to be willing to trade a few cycles for readability and code maintainability

You might want to explore what Brother Bob proposed: Implementing algebraic vectors into your code:

[code]class vector3
{
public: // Make it right
int x;
int y;
int z;
};

// make a method in class world that combines blockGetAmount and blockExists
// the offset seem to be inside world so you don't even need to pass it as an argument.
bool World::BlockValidExists(vector3& v_pos, vector3& v_xof)
{
return (blockExists (v_pos, v_xof) && (blockGetAmount(v_pos, v_xof)!=-1));
}

// and then ...

vector3 xof_vector (world0->xOffset-location.x, world0->yOffset-location.y, world0->zOffset-location.z);
vector3 pos_vector (x-1, y, z);

if (world0->BlockValidExists(pos_vector, xof_vector))
{
[/code]

Share this post


Link to post
Share on other sites
I was just about to mention, before I saw Owls latest post that you're passing exactly the same variables through to World twice asking for two separate results. It would be far more readable if you simply introduced a third function in World that takes the variables once and does the two tests then passes back False if both of the values are true.

You'll learn as you go on, especially with larger projects that if you come back to that code in a couple of months you'll have to spend half an hour just deciphering what it was you were doing with it all. In my opinion readable code is more important even than super efficient code. I know a lot of people will say that when programming for realtime calculations like games efficiency is paramount but readable code will make development easier and quicker which should give you plenty of time at the end for efficiency changes if they're even needed at that point :)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this