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
