I don't think there is any reason to mandate any particular style.  If
that's the way you want to write some code, that's fine with me.

IMO, my brain thinks "loop" when it sees "do" as in, "something in here
may need to be done more than once", so if you do use this style, could
you try to remember to add a comment after the do?

do // while-false
{

Note that once you reversed the logic of the if conditions, you could also
code this section as an if-else chain.

if (prompt == null);
else if (prompt == "");
else if (!skin);
else if (!skin.currentState);
else if (skin.currentState.indexOf("WithPrompt") == -1 && text.length !=
0);
else if (skin.currentState.indexOf("WithPrompt") != -1 && text.length ==
0);
else
  invalidateSkinState();

But to me, just using whitespace seems sufficient:

if (!((prompt == null) ||
      (prompt === "") ||  // this code needs to be tested if the prompt is
"0"
      (!skin) ||
      (!skin.currentState) ||
      (skin.currentState.indexOf("WithPrompt") == -1 && text.length != 0)
||
      (skin.currentState.indexOf("WithPrompt") != -1 && text.length == 0)))

    invalidateSkinState();

Or the original reformatted with white space:

if (prompt != null &&
    prompt != "" &&
    skin && 
    skin.currentState &&
    (skin.currentState.indexOf("WithPrompt") != -1 && text.length != 0 ||
     skin.currentState.indexOf("WithPrompt") == -1 && text.length == 0))
    invalidateSkinState();


There is very little optimization in the compiler, so you have to
hand-optimize your code.  A smart compiler might not set up the do loop,
but our compilers will.  But that would rarely affect performance and only
adds a few bytes per usage, although it could add up for 1000's of usages.
 A smart compiler or smart runtime might inline the function that you
could wrap all the conditions into as some suggested because if it turns
out to be inside an important loop, you wouldn't want to pay for the
function call overhead.

Anyway, always interesting to see different code approaches.

-Alex

On 11/19/15, 11:08 AM, "Harbs" <harbs.li...@gmail.com> wrote:

>To be clear, I think Mark’s code was a nice compromise between approach
>#1 and approach #2.
>
>In this case following Flex code formatting style, do-while-false should
>look something like this:
>
>do
>{
>   if(prompt == null)
>        break;
>
>   if(prompt == "")
>        break;
>
>   if(!skin)
>        break;
>
>   if(!skin.currentState)
>        break;
>
>   if(skin.currentState.indexOf("WithPrompt") == -1 && text.length != 0)
>        break;
>
>   if(skin.currentState.indexOf("WithPrompt") != -1 && text.length == 0)
>        break;
>
>   invalidateSkinState();
>} while(false);
>
>An additional advantage here is that you can break up the OR statement
>into two separate conditions.
>
>I’m also not proposing using this pattern all the time. Sure. For a few
>simple conditions if(a && b) is the simplest way to go. There are times
>when nested ifs make sense as well. If there’s a set of conditions which
>are used in a number of places, a function call like Justin mentions make
>the most sense there.
>
>On Nov 19, 2015, at 3:19 PM, Maxim Solodovnik <solomax...@gmail.com>
>wrote:
>
>> 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 <
>> mark.kessler....@usmc.mil> 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:harbs.li...@gmail.com]
>>> Sent: Thursday, November 19, 2015 3:16 AM
>>> To: dev@flex.apache.org
>>> 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
>

Reply via email to