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 <[email protected]> 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 <[email protected]> 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 <
>> [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
>
