in reply to Re: bloated 'if' formatting
in thread bloated 'if' formatting

From a maintainance perspective, this is the very worst thing to do. You are creating the situation whereby the setting of your flags becomes divorced from the if statement through the injection of code or during refactoring.

In your second version you have created at least half a dozen chances for errors to creep in that are not present in the original compound statement.

Take a browse at Code Smell and you'll find that you are violating many of those principles. They are only hints and guidelines, and in the end it is your code and your judgement, but you might find the read interesting anyway.


Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
The "good enough" maybe good enough for the now, and perfection maybe unobtainable, but that should not preclude us from striving for perfection, when time, circumstance or desire allow.

Replies are listed 'Best First'.
Re^3: bloated 'if' formatting
by revdiablo (Prior) on Oct 06, 2005 at 18:36 UTC
    You are creating the situation whereby the setting of your flags becomes divorced from the if statement through the injection of code or during refactoring.

    This is a risk you always take when you break something up into multiple statements. If you can't rely on later development to keep related lines of code together, then there are greater issues to worry about. I think writing simple, clear code is one of the best ways to help maintenance, and shoving a complex condition into one statement is not always simple and clear.

    That said, I'm not entirely sure the examples I used -- or those given in the original post -- are quite complex enough to benefit from this type of thing. That's why they're examples, and not actual code. There comes a point when the purpose of a condition gets lost amongst the noise, though, and that's when my advice comes into play.

    Take a browse at Code Smell and you'll find that you are violating many of those principles.

    I'm curious which code smell you think I'm, er, smelling of? They cover quite a wide range of different problems, so it's a bit difficult to talk about "code smells" as a whole.

      Okay, I agree that they are only (very cursory) examples, but I'm really talking to the principle of what you are suggesting.

      One of the smells I was thinking of is DRY (Don't Repeat Yourself) Principle states:

      Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

      This is often mistaken/misinterpreted as a variation on OnceAndOnceOnly, but it is quite different (IMO). Using your example (whilst recognising it was only an example :):

      my $first_foo = foo1 && foo2; my $second_foo = foo3 && bar1 && baz1; my $foo_is_ready = $first_foo || $second_foo;

      And assuming (for the sake of discussion, and noting the absence of sigils) that foo1, foo2, foo3, bar1, baz1 are function/methods returning boolean values.

      At the point of invokation, those methods are returning their result based upon the internal state of their objects. By temporarially storing the (composite) values of the methods into variables whilst deferring the decision based upon them, you create a window for opportunity for the internal state of one or more of those objects to change, through injected code, delay or some other mechanism.

      You have also created new, named, compound states. If bar1 becomes redundant, then whatever well chosen compound name (symbolised by your $second_foo), that you chose to represent that compound state, is now wrong; and you have to not only remove the reference to bar1, but also rename $second_foo and all the subsequent places that reference it, so creating several potential maintanence problems along the way.

      Whereas, leaving the compound state entirely within the compound statement:

      if( foo1 && foo2 or foo3 && bar1 && bar2 ){ ... }

      If laid out in a clear manner according to your preference or site standards, avoids all of these potential problems.

    • It is very hard to inject code accidently.
    • If bar1 becomes redundant, you just delete it and the associate &&, and it is done with.

      If the combined conditional states need further clarification, then comments are a better way:

      if( foo1 && foo2 ## First foo or foo3 && bar1 && bar2 ## Or second foo ){ ... }

      My own personal take on this is brevity == clarity. I find you rarely clarify anything when you add to to it's complexity, whether code or speech or writing (though as everyone here knows, the latter certainly isn't my forte).

      Every now and again over the years I've encountered one of those people that comes to a meeting and listens to everyone else waffle on and then stands up and summaries all the issues in 2 or 3 sentences, usually of 10 short words or less. How I envy those people. They also tend to write reports 1/3rd the length of everyone elses and still say twice as much with 5 times the impact.

      It's not about writing the shortest or the cleverist code. It's about doing just what needs to be done without adding anything that isn't needed.

      "Only doing what needs to be done" is my take on Premature Generalisation, which is a little out of context here, but is still relevant if you consider your "used once states" in the same light as "used once functions" and "used once libraries". They do have their place and uses, but they should not be used without careful consideration.


      Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
      Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
      "Science is about questioning the status quo. Questioning authority".
      The "good enough" maybe good enough for the now, and perfection maybe unobtainable, but that should not preclude us from striving for perfection, when time, circumstance or desire allow.

        I'm not even going to try and reply to most of your post, because it clearly and articulately describes your feelings about this situation, and frankly I think you're mostly right. I agree especially with this part:

        They do have their place and uses, but they should not be used without careful consideration.

        That sounds right, and I probably should have included much the same disclaimer in my original post. I didn't really mean to recommend making used-once-states for every condition that has more than 2 elements. Perhaps it was inappropriate that I brought it up in the context of the original post, but it seemed like an idea that others would find interesting and useful, and I thought was somewhat relevant.

        At the point of invokation, those methods are returning their result based upon the internal state of their objects. By temporarially storing the (composite) values of the methods into variables whilst deferring the decision based upon them, you create a window for opportunity for the internal state of one or more of those objects to change, through injected code, delay or some other mechanism.
        This sound very profound, but if you'd always follow this reasoning, you'd never break down complex statements into multiple statements, or for that matter, use subroutines.

        Programming is about making trade-off. Yes, you create a window of opportunity for things to change. But your gain is simpler, easier to maintain code. It's a price I'm usually willing to pay.

        Perl --((8:>*