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

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.

Replies are listed 'Best First'.
Re^4: bloated 'if' formatting
by BrowserUk (Patriarch) on Oct 06, 2005 at 20:25 UTC

    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:>*
        This sound very profound, but if you'd always follow this reasoning, you'd never break down complex statements into multiple statements, ...

        And that sounds like a good refutation, but it misses the point completely.

        If you understand some of the thinking behind Functional Programming (FP) and referential transparency, you'll see that what I'm suggesting is very similar. In (some variants of) FP, you cannot save composite values into variables as there is no assignment. It's proponents will tell you that this leads to safe, provably correct code.

        I won't go that far, but I do suggest that avoiding the duplication of state where possible--and especially where that duplication creates 'used once intermediary variables' is Very Good Practice.

        I realise that you see this "simplification" but believe to be 'complexification'.

        Either way, you have the same number of unavoidable clauses and states. Spreading them out over several statements, and introducing addition states in order to do so, does not simplify things. All the original clauses and states are still there, and you've added new ones to the mix. In the process you have spread those clauses and states far apart and made it harder to see the overall picture.

        Have you ever tried to navigate your way across a strange city using one of those A5 sized "A-Z" style maps? If so, you'll recall how disorienting it is to see only a very localised picture! And how easy it is to find yourself heading in completely the wrong direction because you don't have the overall picture.

        Sheet maps may be less convenient to handle, but the ability to see both ends of the journey at the same time, or even just the overall direction in which you are going, makes it much easier to avoid getting sent off in entirely the wrong direction by some town planners idea of the perfect one-way system.

        As a many times maintenance programmer, of both my own code and that collectively written by dozens of people working thousands of miles away on different continents, I'd much rather spend 5 minutes understanding a single compound statement, than 1 minute on each of 10 statements and another 10 minutes flicking up and down the file, or between files, trying to understand how each of those 10 "simple" statements fit together.

        The problem with using lots of simple statements to implement what can (reasonably and clearly) be achieved with one (or two, or three) compound statements, is that it enables, and actively encourages, the maintenance programmer to dive straight into the middle of the indivisible group and understand the action of a single statement, without recognising the effect that action has on the other statements around it. Or the affect that making the "simple change" that seems obvious, will have upon them, as a group.

        The great benefit of high and very high level languages, and their ability to do a lot in a few statements, comes not (directly) from the reduction in typing. It comes from the fact that code that performs those few statements is used in many contexts. And, as a result, is exercised (system and integration tested as opposed to unit tested) by many different people. Many, many times.

        This means that the bulk of the work is being done by very well exercised code, with each application benefiting from the testing, explicit or implicit, that is done by every previous application that used the same statements.

        The simplest way to throw away those benefits is to avoid using the facilities that the language provides.

        ... or for that matter, use subroutines.

        That's a very wrong inference to draw from what I've said, or from the references I posted.

        I have no problem at all in using function or subroutines, but I do suggest that 'used once subroutines' should be used sparingly and with care.

        A function that is re-used, as part of a library say, very quickly gets used in many different circumstances. It's interface and documentation get exercised by multiple programmers. And it get called in many contexts. This quickly highlights any assumptions made by the programmer, and any close couplings it may have on context.

        Used once subroutines do not get this real-world exercising, and no amount of unit testing is a substitute. This means that the assumptions and contextual coupling do not become apparent until someone comes along and changes one end or the other without the benefit of the overview the original programmer had when he wrote them. That is when things start to go wrong--in horribly subtle ways.

        In FP, the F stands for Function(al), and subroutines (in Perl) are all functions. FP uses them in abundance. In that way, all intermediary values becomes automatically managed, anonymous values on the stack. Both those thing are important.

      • As they are anonymous, the programmer cannot refer to them explicitly, so there is no scope for him to inject code in the wrong place or modify their value or misinterpret their name or use.
      • The automatic management means that they come into being at just the right time; exist for just as long as they are needed; and go away when the aren't. Their scope is defined by structure of the code, not some arbitrary (mis)placement by the programmer.

        Maybe you are young and haven't been bitten enough times yet. Maybe you're just very good and never make the mistakes. Maybe you haven't had to share your code with others that aren't as careful as you.

        As always, I'm not advocating my preferences for anyone else. They evolved over time through (my) experience(s) and are what work for me. I set some store by reading reputable people and sites that confirm my conclusions, but I often don't modify my own practices solely because someone else says they do things differently--even when they are luminaries and/or offer sound reasoning to support their case--if it conflicts with my own experiences and conclusions.


        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.