I believe example2 need to be reformatted to match example1
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);
and most probably second if need to divided into 2 ifs :)
On Thu, Nov 19, 2015 at 6:51 PM, Kessler CTR Mark J <
[email protected]> wrote:
> 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
>
--
WBR
Maxim aka solomax
