Let me see if I can reformat into all the examples so we can have a real sample
for people. Below shows the original, what was committed. Then shows the last
few examples. Hopefully this won't get word wrapped to badly or have font
issues.
The original was changed to better group the sets of conditions and added
brackets to be able to see the "do something" separated from the conditions
better. My input for it would be; what you choose will be based on the current
code and will vary with each case.
Original:
if (prompt != null && prompt != "" && skin && skin.currentState &&
(skin.currentState.indexOf("WithPrompt") != -1 && text.length != 0 ||
skin.currentState.indexOf("WithPrompt") == -1 && text.length == 0))
invalidateSkinState();
Commited:
if (prompt != null && prompt != "" && skin && skin.currentState)
{
if (skin.currentState.indexOf("WithPrompt") != -1 && text.length != 0 ||
skin.currentState.indexOf("WithPrompt") == -1 && text.length == 0)
{
invalidateSkinState();
}
}
example 1:
if (prompt != null)
{
if (prompt != "")
{
if (skin)
{
if (skin.currentState)
{
if (skin.currentState.indexOf("WithPrompt") != -1 &&
text.length != 0 ||
skin.currentState.indexOf("WithPrompt") == -1 &&
text.length == 0)
{
invalidateSkinState();
}
}
}
}
}
example 2:
do
{
if(prompt == null){break};
if(prompt == ""){break};
if(!skin){break};
if(!skin.currentState){break};
if(skin.currentState.indexOf("WithPrompt") == -1 && text.length != 0 ||
skin.currentState.indexOf("WithPrompt") != -1 && text.length ==
0){break};
invalidateSkinState()
break;
} while(false);
-Mark
-----Original Message-----
From: Harbs [mailto:[email protected]]
Sent: Thursday, November 19, 2015 3:16 AM
To: [email protected]
Subject: do-while-false
There’s a coding pattern that I like to use which I picked up from the InDesign
SDK. When there’s some code which needs a lot of conditions to be executed,
it’s hard to write the conditions in a way that’s easily human readable.
You can either do:
if(conditiona && conditionb && conditionc &&conditiond(
{
//do something
}
or:
if(conditiona){
if(conditionb){
if(conditionc){
if(conditiond){
// do something
}
}
}
}
Both of these are kind of hard on the eyes and make fixes error-prone.
The do-while-false solution is much more ledgible than both of these and it
goes like this:
do{
if(!conditiona){break};
if(!conditionb){break};
if(!conditionc){break};
if(!conditiond){break};
//do something
}while(false);
The reason it works is that do-while-false executes exactly once, and break
leaves the “loop”.
The pattern reverses the logic and instead of checking when you *should*
execute the code, it checks when you should bail out. The reason I like this
pattern is because it makes for much flatter code and each condition stands on
its own. That makes it easy to read and fix conditions at a later point.
How do folks feel about trying to use this pattern?
What prompted this post is commit b29975c which attempts to make a mess of
conditions for invalidateSkinState() a bit clearer.
Thoughts?
Harbs
