GML Why Yin-Yang if Blocks suck

FrostyCat

Member
Why Yin-Yang if Blocks suck

GM Version: N/A
Target Platform: All
Download: N/A
Links: N/A

Overview
This article aims to document the rookie anti-pattern of adjacent opposite if blocks (i.e. if (xyz) { ... } if (!xyz) { ... }), its pitfalls and what to use instead of it.

What are Yin-Yang if Blocks?
Many novices write "if A do X, otherwise do Y" behaviours in the following form, with 2 adjacent if blocks each carrying the condition and its manually inputted negation respectively:
Code:
if (condition) {
    //...
}
if (!condition) {
    //...
}
There are also chained versions of this pattern with 3 or more adjacent if blocks checking the same value. Example:
Code:
if (value == 0) {
    //...
}
if (value == 1) {
    //...
}
if (value == 2) {
    //...
}
These anti-patterns all fall under the term "Yin-Yang if Blocks," which I will refer to for the rest of this article. They are characterized by the following:
  • The if blocks contain multiple attempts to check the same value or condition.
  • The if blocks are arranged with the intent of mutual exclusion (i.e. only one of them is meant to execute on any single run), but contain no else.

Why are Yin-Yang if Blocks bad?

Reason 1: They fail at mutual exclusion.

Many novices use Yin-Yang if Blocks with the hope that only one of the block bodies will execute on any given run. But when one block's body operates in a way that it enables some subsequent condition, this false hope becomes a big problem.

A particularly common example of such novice code is this:
Code:
if (variable == true) {
    variable = false;
}
if (variable == false) {
    variable = true;
}
A novice might think this flips variable between true and false. But a quick trace reveals a different conclusion:
  • variable starts out false: The first block doesn't run, second block makes it true. So far so good.
  • variable starts out true: The first block runs and makes it false. Then the second block runs because the first block made it false, and makes it true again.
There is a reason why asking what this particular piece does is a common weed-out question for junior-level technical interviews. Answering the novice way not only tells the employer you are green enough to have never seen or fallen for it before, it also tells the employer that you can't follow code the way a computer does. Neither of these are desirable coder qualities.

Reason 2: It is easy to negate the expression improperly.

The 2-block form is especially prone to problems when && or || are part of the condition, and DeMorgan's Laws are improperly applied. Here is an example:
Code:
if (place_meeting(x, y+1, obj_ground) || place_meeting(x, y+1, obj_box)) {
    ysp = 0;
}
if (!place_meeting(x, y+1, obj_ground) || !place_meeting(x, y+1, obj_box)) {
    ysp = 5;
}
This has the unwanted effect of pulling the player through ground blocks and boxes, unless the player has both a ground block and a box immediately under foot.

The chained form is also often done improperly, especially with ranges. Here is an example:
Code:
if (score <= 100) {
    audio_play_sound(snd_yousuck, 1, false);
}
if ((score >= 100) && (score <= 200)) {
    audio_play_sound(snd_couldbebetter, 1, false);
}
if (score >= 200) {
    audio_play_sound(snd_yourock, 1, false);
}
An exact score of 100 or 200 will trigger two sounds at once.

As you will see shortly, properly done alternatives to Yin-Yang if Blocks leave no room for amateurish human errors like these.

Reason 3: It wastes time evaluating the same expression twice.

While Yin-Yang if Blocks don't always result in tangible bugs, they always duplicate effort. This might not matter much for a few arithmetic comparisons, but for more expensive functions such as collision checks against precise masks, the performance penalties can start to add up, especially if many instances are running those checks at once.

Reason 4: It tells everyone you have no clue how to code.

Because of the pitfalls listed above and how easy they are to avoid with basic techniques, Yin-Yang if Blocks are commonly considered a mark of the uninitiated. Using one not only puts your project at risk of bugs, it also tells others reading your code that you don't know what you're doing.

Competent Alternatives to Yin-Yang if Blocks

Alternative 1: else

For "if A do X, otherwise do Y" behaviours, state only the positive version of the condition, and leave the negative to else.
The else keyword ensures that the second block never runs if the first block gets to, even if code in the first block makes the condition false in the process.

Code:
if (condition) {
    //...
} else {
    //...
}
Code:
if (place_meeting(x, y+1, obj_ground) || place_meeting(x, y+1, obj_box)) {
    ysp = 0;
} else {
    ysp = 5;
}
else can also form ladders with multiple if conditions to make them mutually exclusive. When writing subsequent conditions, remember to take advantage of what getting to the point of else has already ruled out.

Code:
if (score <= 100) {
    audio_play_sound(snd_yousuck, 1, false);
} else if (score <= 200) {
    audio_play_sound(snd_couldbebetter, 1, false);
} else {
    audio_play_sound(snd_yourock, 1, false);
}
Alternative 2: switch

When comparing one source value to many single target constants (and up to one optional no-match case), use a switch block.
Code:
switch (value) {
    case 0:
        //value is 0
        break;
    case 1:
        //value is 1
        break;
    case 2:
        //value is 2
        break;
    default:
       //no match
        break;
}
Alternative 3: ! (for toggling variables)

For toggling a variable between true and false, use ! (the Boolean not operator).
Code:
variable = !variable;
The ! operator returns true if the operand is false, and false if the operand is true. This effectively flips the given value.

Alternative 4: Arithmetic operations (for incrementing values)

Whenever feasible, try to relate a variable to its future value using arithmetic expressions, instead of case-by-case handling.

Incrementing a variable by 1 (e.g. 0, 1, 2, 3, 4, ...):
Code:
variable++;
Incrementing a variable by N (e.g. for N=2: 0, 2, 4, 6, 8, ...):
Code:
variable += N;
Loop-arounds should also be done using arithmetic expressions, and ideally rooted at 0 instead of 1. Like most counting patterns in computer science, 0 is a more sensible starting point than 1, allowing you to use simpler expressions for the task than if you insist on layman one-indexing approaches.

Incrementing a variable by M and looping around N (e.g. for M=1, N=4: 0, 1, 2, 3, 0, 1, 2, 3, ...)
Code:
variable = (variable+M) mod N;
Decrementing a variable by M and looping around N, with M<N (e.g. for M=1, N=4: 0, 3, 2, 1, 0, 3, 2, 1, ...)
Code:
variable = (variable+N-M) mod N;
 

Alice

Toolmaker of Bucuresti
Forum Staff
Moderator
@FrostyCat I agree that Yin-Yang if Blocks - used with the intent of mutual exclusion - are invalid, and the tutorial does a good job explaining the problems with them, as well as presenting the alternatives.

I wonder, would you consider this code a valid exception to this rule? I would sometimes do something similar for grid-based movement code:
GML:
if (!is_moving) {
    // check if player initiated a valid move, and changes "is_moving" to true if that's the case
    player_check_move_input();
}
if (is_moving) {
    // gradually moves the player sprite to the next cell, and changes "is_moving" to false when the next cell is reached
    player_continue_move();
}
In this case, the code in second "if" is meant to respond immediately to the player initiating move in the first "if". If it was "if/else" instead, there would be one frame lag, especially noticeable when the player moves continuously in some direction.

If you think it's a valid counter-example, I think it would be good to mention it in the tutorial. It would further emphasize that the principle applies when if's are meant to be mutually exclusive, as opposed to when we expect and want multiple if's to execute sometimes (here: when the player begins a move).
 

rytan451

Member
@Alice I wouldn't recommend doing that. It has huge readability issues, and can lead to bugs.

Your method, player_check_move_input() changes is_moving, but a quick read of the code would not reveal this. You'd have to remember that the method changes is_moving, which is a tall order when you have a hundred methods, each of which can each change a handful of variables.

Even if the method were to be inlined, this still is a problem. When you're browsing through code (up to 10K lines of code), you don't want to have to read all the code. Therefore, you'd probably collapse most of the if statements. This makes you see:

GML:
if (!is_moving) {...}
if (is_moving) {...}
If you read that code, then (since you can't remember all your code) you might be tempted to refactor it, so that it would read like this:

GML:
if (!is_moving) {..}
else {...}
This would, of course, lead to a bug.

It is for this reason that I cannot recommend doing that.

When you reach 10K lines of code, I guarantee that you will either be cursing your past self for writing unreadable code, or thanking your past self for spending the effort to make the code easily readable.
 

Alice

Toolmaker of Bucuresti
Forum Staff
Moderator
Hmm, I can sorta see where you're from, but in practice - out of various pieces of code I've written and later didn't remember about - I never had that trouble with this specific double-if.

I suppose the code is pretty characteristic to me, maybe because it's this special case where double-if shouldn't be replaced one-to-one with if/else.
This "process input if nothing blocks it, then process movement/tweening/whatever if there's one ongoing" is so ingrained in my memory that I'd easily recognise it whenever I would see that. The fact that I remember this specific exception when reading this article and pointed it out kinda proves that.

If anything, I'd be more concerned about *others* reading my code - I admit this could be a valid concern, but the problem probably could be circumvented by a short comment explaining the double-if is intended.

Also, while it's not stated outright that is_moving variable is changed in player_check_move_input(), I believe both names using "move" and player_check_move_input() being the only method give at least some indication of connection.
 
I think I must have an exception to this somewhere in my code, for instance, I think there are probably times as with the OP score range example, where you might intentionally want more than one thing to happen. However, it is with all advice like this I think: if you know the reasons why you wouldn't want to write code like this, then you can make educated exceptions when the rare case does appear.

The counter to even that though is still the readability question, so I think it's advisable when making an exception to make sure your comments are extra clear.
 

Joe Ellis

Member
@Alice I think that kind of exception is too complex, it mainly involves what's in the other two functions and it'd just confuse beginners (Rookies haha) and make the message(s) of the post less coherent. I like this post as it is, everything follows on nicely and is easy to understand. That thing would just throw a spanner in the works and make dem rookies be questioning all types of bs
None of it's completely rigid\textbook though of course, when you know what you're doing you can bend the rules a lot, but it only works if you know exactly what's going on, so I think the op as it is is the best advice for dem darn rookies :D
 

Alice

Toolmaker of Bucuresti
Forum Staff
Moderator
None of it's completely rigid\textbook though of course, when you know what you're doing you can bend the rules a lot, but it only works if you know exactly what's going on, so I think the op as it is is the best advice for dem darn rookies :D
I can see your point.

My concern stems from my programming practices research I do sometimes, and also direct interactions with other programmers.
They'd often say that "XYZ is evil", specifically understanding it as an universal truth or creating an impression it is such. Yet when asked about why "XYZ is evil", they'd either fail to provide practical reasons or cite those that don't apply to intended use-case.

This makes researching programming practices somewhat more difficult, because then it's hard to tell whether:
- the intended use-case is fine because the cited problems don't apply
- the intended use-case still has other problems I don't know about (but some others do) that will bite me in the back later
Then I'd need to decide whether I should:
- risk following through with the intended use-case
- write larger code to satisfy the rule
- spend time doing more research to find out if someone described a situation closer to my intended use-case, whether as an exception to the rule or a still-bad practice.

It can also become troubling when you know your use-case is valid - maybe even preferable - but then you need to spend time explaining it to other programmers who are stuck in the "XYZ is evil" mindset, because whoever taught them the rule failed to mention there are rare exceptions.

And yet, "It depends" isn't an answer to everything - there are certain rules that near-universally result in higher-quality code, such as identing the code inside the block *somehow* or not putting entire logic of any remotely complex application in a single class/object. It makes the codescape even trickier to navigate, with some genuinely near-universal rules and other mostly-applicable-with-known-exceptions rules.
If you describe the known-exceptions rule without pointing out the exceptions, people might think of it as a near-universal rule instead. They'd sometimes wrestle with code trying to follow it instead of opting for a simpler acceptable break, or questioning other programmers who use the acceptable break use-case.

So I guess while mentioning exceptions might confuse the rookies, not mentioning them might confuse experienced programmers. ^^'
(also, give them a hard time with rookies who still believe "XYZ is evil" is an universal truth)
 

Joe Ellis

Member
I've always just learned everything myself and taken anything I read online with a pinch of salt, still keep it in mind but judge the problem
yourself. Sometimes things other people have said help you understand better, but it's been very rare for me. It's normally just hearing one person's understanding of it, which is different to yours, so it's a bit like going into their world while you're reading it. Then you come out mostly thinking "ughh" (even if you have learnt some of the vital information you were looking for).
They're (A lot of the time, but not always of course) distorted\exaggerated based on their own problems with understanding certain elements, and in some ways it's a bit poisonous for all the people(especially beginners) reading about it.
OR influential, then thousands of people start doing stuff based on one "blogger's" thorough explanations. Then thousands of forum posts starting with some code they copied and pasted from them.
If the writer was doing a good job, they would've explained it in a way that even people who have never touched a computer would be able to understand, simply from the logic.

But you have to remember that code and the way it works is a lot less complex than the human mind, so people often explain or understand it in a much more complex way than it actually is.

I think the most important thing is to always understand what the code you're writing is making the computer do. Then it's just a case of pure logic, rather than categorizing different parts of the coding process and what is the correct thing to do at each time. The ideal way is to just know what to do, the same as, hmmm this guy:

https://www.youtube.com/c/EvolutionPrimitiveTime/videos
 

Bart

WiseBart
Interesting discussion on that suggested exception.
I wouldn't do it like that for the same reasons rytan451 mentions: doing it like that gives you no idea of what might happen behind the scenes.
Why not just return from the function and assign to is_moving:
GML:
// Check #1
if (!is_moving) {
    is_moving = player_check_move_input();
}
// Check #2
if (is_moving) {
    is_moving = player_continue_move();    // we can see that is_moving may have changed, so we may end up here
}

I find the list in the OP to be an interesting overview and I'd like to make a suggestion myself. There is no mention of the ternary operator ?
I'd consider it to be a useful addition, i.e. it could work well for the example in reason 2:
GML:
if (place_meeting(x, y+1, obj_ground) || place_meeting(x, y+1, obj_box)) {
    ysp = 0;
}
if (!place_meeting(x, y+1, obj_ground) || !place_meeting(x, y+1, obj_box)) {
    ysp = 5;
}
Becomes:
GML:
var condition = (place_meeting(x, y+1, obj_ground) || place_meeting(x, y+1, obj_box));
ysp = condition ? 0 : 5;
 
Top