Skip to main content
deleted 246 characters in body
Source Link
Rei Miyasaka
  • 4.6k
  • 1
  • 35
  • 36

Make boolean variables for each small steps:

bool step1 = i + u == b || q >= a; bool step2 = a != b && p.isGood() && group1; bool step3 = group2 || k.isSomething() || m > n; if (step3) { doSomething(); } 

This is of course similar to Lacrymology's answer, except with different names for each step.

If you name step1, step2 and step3 in ways that make good conceptual sense, this should be by far the most legible. p.isGood() and k.isSomething() may sometimes be invoked in situations where it wouldn't be in your original code, so this wouldn't be an option if those functions are expensive or if you're running this code in a very tight loop.

On the other hand, you needn't worry about the performance hit that creating new variables might incur; a good compiler will optimize them out.

An example with rectangle collision detection (which you probably wouldn't use due to the aforementioned performance hit):

if((a.x + a.width >= b.x || b.x + b.width >= a.x) && (a.y + a.height >= b.y || b.y + b.width >= a.y) ) { collision(); } 

Might become:

bool horizMatch = a.x + a.width >= b.x || b.x + b.width >= a.x; bool vertMatch = a.y + a.height >= b.y || b.y + b.width >= a.y; if(horizMatch && vertMatch) { collision(); } 

Personally, I would avoid refractoring these conditions out into separate functions. It'd be silly to test such small functions in isolation, and having the definitions dangling outside the function makes the code slightly less self-apparent.

Also, if you want to leave your code as is, I think that would be totally fine too. I honestly think your code is quite legible. Obviously I don't know what exactly a b x y i u p k m n are, but as far as structure goes, it looks good to me.

Make boolean variables for each small steps:

bool step1 = i + u == b || q >= a; bool step2 = a != b && p.isGood() && group1; bool step3 = group2 || k.isSomething() || m > n; if (step3) { doSomething(); } 

This is of course similar to Lacrymology's answer, except with different names for each step.

If you name step1, step2 and step3 in ways that make good conceptual sense, this should be by far the most legible. p.isGood() and k.isSomething() may sometimes be invoked in situations where it wouldn't be in your original code, so this wouldn't be an option if those functions are expensive or if you're running this code in a very tight loop.

On the other hand, you needn't worry about the performance hit that creating new variables might incur; a good compiler will optimize them out.

An example with rectangle collision detection (which you probably wouldn't use due to the aforementioned performance hit):

if((a.x + a.width >= b.x || b.x + b.width >= a.x) && (a.y + a.height >= b.y || b.y + b.width >= a.y) ) { collision(); } 

Might become:

bool horizMatch = a.x + a.width >= b.x || b.x + b.width >= a.x; bool vertMatch = a.y + a.height >= b.y || b.y + b.width >= a.y; if(horizMatch && vertMatch) { collision(); } 

Personally, I would avoid refractoring these conditions out into separate functions. It'd be silly to test such small functions in isolation, and having the definitions dangling outside the function makes the code slightly less self-apparent.

Also, if you want to leave your code as is, I think that would be totally fine too. I honestly think your code is quite legible. Obviously I don't know what exactly a b x y i u p k m n are, but as far as structure goes, it looks good to me.

Make boolean variables for each small steps:

bool step1 = i + u == b || q >= a; bool step2 = a != b && p.isGood() && group1; bool step3 = group2 || k.isSomething() || m > n; if (step3) { doSomething(); } 

This is of course similar to Lacrymology's answer, except with different names for each step.

If you name step1, step2 and step3 in ways that make good conceptual sense, this should be by far the most legible. p.isGood() and k.isSomething() may sometimes be invoked in situations where it wouldn't be in your original code, so this wouldn't be an option if those functions are expensive or if you're running this code in a very tight loop.

On the other hand, you needn't worry about the performance hit that creating new variables might incur; a good compiler will optimize them out.

An example with rectangle collision detection (which you probably wouldn't use due to the aforementioned performance hit):

if((a.x + a.width >= b.x || b.x + b.width >= a.x) && (a.y + a.height >= b.y || b.y + b.width >= a.y) ) { collision(); } 

Might become:

bool horizMatch = a.x + a.width >= b.x || b.x + b.width >= a.x; bool vertMatch = a.y + a.height >= b.y || b.y + b.width >= a.y; if(horizMatch && vertMatch) { collision(); } 

Also, if you want to leave your code as is, I think that would be totally fine too. I honestly think your code is quite legible. Obviously I don't know what exactly a b x y i u p k m n are, but as far as structure goes, it looks good to me.

Source Link
Rei Miyasaka
  • 4.6k
  • 1
  • 35
  • 36

Make boolean variables for each small steps:

bool step1 = i + u == b || q >= a; bool step2 = a != b && p.isGood() && group1; bool step3 = group2 || k.isSomething() || m > n; if (step3) { doSomething(); } 

This is of course similar to Lacrymology's answer, except with different names for each step.

If you name step1, step2 and step3 in ways that make good conceptual sense, this should be by far the most legible. p.isGood() and k.isSomething() may sometimes be invoked in situations where it wouldn't be in your original code, so this wouldn't be an option if those functions are expensive or if you're running this code in a very tight loop.

On the other hand, you needn't worry about the performance hit that creating new variables might incur; a good compiler will optimize them out.

An example with rectangle collision detection (which you probably wouldn't use due to the aforementioned performance hit):

if((a.x + a.width >= b.x || b.x + b.width >= a.x) && (a.y + a.height >= b.y || b.y + b.width >= a.y) ) { collision(); } 

Might become:

bool horizMatch = a.x + a.width >= b.x || b.x + b.width >= a.x; bool vertMatch = a.y + a.height >= b.y || b.y + b.width >= a.y; if(horizMatch && vertMatch) { collision(); } 

Personally, I would avoid refractoring these conditions out into separate functions. It'd be silly to test such small functions in isolation, and having the definitions dangling outside the function makes the code slightly less self-apparent.

Also, if you want to leave your code as is, I think that would be totally fine too. I honestly think your code is quite legible. Obviously I don't know what exactly a b x y i u p k m n are, but as far as structure goes, it looks good to me.