Of course there’s no reason not to mix and match. In this case, I’d probably 
join similar conditions like this:

do
{
  if(prompt == null || prompt == "")
       break;

  if(!skin || !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);

On Nov 19, 2015, at 9:08 PM, 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