7

If createWorld() is really long and I need to split it, I can split it to createLight(), createEarth(), createPlants(), and createAnimals().

So, naturally I do:

function createLight(){
    //work 1
}
function createEarth(){
    //work 2
}
function createPlants(){
    //work 3
}
function createAnimals(){
    //work 3
}

function createWorld(){
    createLight()
    createEarth()
    createPlants()
    createAnimals()
}

But I see a lot of newer developers do what I am tentatively calling "the Stairstep antipattern". It goes something like:

function createWorld(){
    //create light
    //work1
    createEarth()
}
function createEarth(){
    //work 2
    createPlants()
}
function createPlants(){
    //work 3
    createAnimals()
}
function createAnimals(){
    //work 4
}

I am sure this is worse, but I don't think that my opinion alone can sway my colleagues. I think that if one step is createButterfly() then that function has no business calling createGrasshopper() at the end just because that's the "next step".

And even if you name it createButterflyThenCallCreateGrasshopper(), it's still bad design because you can't test the createButterfly code well, and when you have several steps it gets hard to see where the functions are called.

I call it the Stairstep antipattern because I think of it like this:

|
|createWorld(){
              |
              |createEarth(){
                            |
                            |createPlants(){
                                           |createAnimals()

Am I correct in thinking that this is a bad design? Why is this a bad design?

  • 3
    Those are equally valid in principle. Which one is better depends on the meaning of those functions. – CodesInChaos Dec 5 '16 at 20:55
  • 3
    Tail Calls are something different; they are the means by which recursive functions are converted to iterative functions by the compiler or interpreter. You can name the last method call in a function a tail call, I suppose, simply by virtue of its position, but it really has no intrinsic meaning beyond its use in recursion. – Robert Harvey Dec 5 '16 at 21:34
  • 2
    I don't know that it has an "official" name, but you're right that it's better to make each individual function standalone and call them all top-level, instead of one function calling the next. The reason that it is better is that each function is self-contained, and one function doesn't depend on another for its proper operation, unless the inner function itself embodies some replaceable, re-usable functionality in its own right. I consider what you're describing a "tight coupling" problem. – Robert Harvey Dec 5 '16 at 23:11
  • 2
    While this does not answer your question, I suggest you read this answer of mine to a related question. – user22815 Dec 6 '16 at 0:11
  • 3
    Software developers place far too much emphasis on names, and far little emphasis on concepts, anyway. A concept doesn't have to have a name for it to still be valid, and there are plenty of well-named "patterns" of dubious value. – Robert Harvey Dec 6 '16 at 18:04
18

Besides the obvious fact in case #2 createEarth and createPlants do not what their name pretends what they are doing, I think the major flaw here is the violation of the single level of abstraction principle:

createEarth does some work directly, without any abstraction, and then calls createPlants, which is a separate abstraction doing some additional work. But the latter should be on the same level of abstraction as the piece of work done before. So different levels of abstraction are mixed.

Trying to fix this, one would typically first refactor createPlants from #2 like this:

function createPlants(){
    doWork3()  //work 3
    createAnimals()
}

Now is everything on the same abstraction level, so we can resolve the naming issue: doWork3 should better be renamed to createPlants, and createPlants to createLife, since it creates animals and plants:

  function createLife(){
      createPlants()  
      createAnimals()
  }

I think now its obvious that after some further refactoring steps, one automatically ends up in case #1, which follows the SLA principle: inside of createWorld, each call is on the same level of abstraction. Inside each of the other methods, (hopefully) the work done there is also on one level of abstraction.

Note that technically, case #2 will work, too. The problem starts when one has to change something, when the code has to be maintained or evolved. Case #1 creates mostly independent building blocks, which can be easier understood, tested, reused, extended, or reordered on its own. In case #2 each building block depends on another, which throws all the former advantages over board.

  • 5
    Anonymous downvoter, please, leave a comment! I am sure this is a correct answer, but if I did not explain something well, give me a chance to improve it. – Doc Brown Dec 6 '16 at 8:56
  • 1
    Maybe, the answer could be enhanced with examples of the problems mentioned related to the case #2. These could be a good argument in favor of the case #1, and they would be giving hints about why #2 is not advisable. – Laiv Dec 6 '16 at 12:15
  • 6
    I downvoted too... I read your link. While I think your answer is reasonable, I think that "principle" is essentially BS. e.g. "Loops should ideally contain a single statement (usually a method call)." ??? In the real world, this would be bad for readability and maintainability in many cases. Principle sounds like it was written by someone in an ivory tower. – Brad Thomas Dec 6 '16 at 14:06
  • 1
    Problem is how to design the right sequence with the less coupling possible. SLA might help to reduce the length of the sequence. But not the sequence Itself. Chaining the sequence like case #2 comes up with several shortcommings (like testing and coupling). If you applies the Doc's SLA approach to the whole sequence, you end up with the case #1 as the preferable scenario... In many planets createWorld() have not ended with createAnimals(). So far we know, only the Earth accomplished such sequence – Laiv Dec 6 '16 at 18:33
  • 1
    @PieterB: having methods on the same abstraction level does not necessarily mean they are independend from another, that is a different thing. – Doc Brown Dec 6 '16 at 19:21
3

This is a bad design because it's more tightly coupled than it needs to be.

When your functions are composed into one aggregator, each of them can be used (and tested) in isolation. When CreateEarth calls CreatePlants in the chain, it removes that flexibility from your code. It forces plants to be created, which is the usual process, but what happens when the business comes along and wants an ocean world or a volcano world that don't need plants?

By separating the functions, you can better adapt to this inevitable change.

  • do you know of a "name" for the common pattern of putting function calls at the end in order to "continue to the next step"? That was my original posted question, before the editors changed it to "why is this bad design?" – AwokeKnowing Dec 6 '16 at 16:55
  • No. I've seen it a lot in JavaScript land. And it looks like continuation passing style, but has none of the benefits. I'm unaware of a name for it. – Telastyn Dec 6 '16 at 17:01
2

I'd call it something like "excessive call nesting". The whole point of having a separate method is to break out a single logical piece of the algorithm, ideally a re-usable piece. If it is not functionally distinct or re-usable, consider not breaking it out, unless it would significantly ease readability, and break a much large piece of code spanning several pages into more maintainable components. Usually this can be done in a way such that each piece has a reasonable degree of functional independence, even if not intended to be called from more than one place.

See also When is it appropriate to make a separate function when there will only ever be a single call to said function?

1

One of the biggest issues I see is the strict ordering constraint that #2 places on building a World. What happens when I want to createAnimals() before I createPlants()? What if I decide something needs to be done in between? In the first example it's as simple as moving a single line of code. In the second example I need to rewrite at least 2 methods.

-1

Reasons to go way #1:

  1. Function Call Stack optimization.

  2. Modularity for testability.

  3. Modularity for maintainability.

From Wikipedia on function call stack (seems to be not a big issue with nowadays computers, but still is with microcontrollers and other embedded systems):

"In computer science, a call stack is a stack data structure that stores information about the active subroutinesof a computer program".

-3

It's called "bad naming". A method called createPlants which actually create both plants and animals have a misleading name. Possible it indicates a misunderstanding of function abstraction, since the writer does not realize createAnimals is part of what createPlants does.

When decomposing a large function into multiple smaller functions, it can be done sequentially or hierarchically. Both are completely valid, depending on the semantics of the functions. But each function should have a clear and well-defined purpose which should be reflected in its name.

If it is hard to come up with a natural name for a function, it indicates the purpose of the function is not well-defined. If you need to name a function createAllAnimalsExceptGrashopper() to describe its purpose, the functions are not decomposed in a natural way.

But there is nothing wrong per se with calling the "next step" at the end of a function. For example if createEarth has be called before createPlants can be called, I would argue it is better design to have createEarth call createPlants rather than having them called sequentially at the top level. (Possibly the earth could be a parameter to createPlants to make the dependency explicit.)


As for the name of this "antipattern": I think it is misguided to think that any particular instance of bad design must have a specific name. I blame the education system for placing to much importance on knowing and remembering precise terminology, rather than actually understanding the underlying issues. Trying to teach people they shouldn't write "stairstep metods" without them understanding the underlying issue will just cause cargo cult programming.

  • 1
    I agree, but I think the issue is deeper. If you renamed it to createPlantsThenCallNextItem it would be the right name but same problem: it's hard to see the sequence of steps, and you can't easily skip / add steps, and you can't just test the one function – AwokeKnowing Dec 5 '16 at 22:36
  • @AwokeKnowing: ..callNextItem should only make sense as a name if the function did not know what the "next item" is, ie. if the next item was passed as a parameter. If it is hardcoded that the next item is createAnimals, then the sensible name would be createPlantsAndAnimals. – JacquesB Dec 6 '16 at 6:58
  • @JacquesB but if then createAnimals() chains into createFoodChain() do we continue to add on to createPlantsAndAnimalsAndFoodChain()? If not, we're doing steps that aren't described in the function- but if we are, every step must have every other step outlined in the name, and I'm refactoring a codebase if I ever come across a function named createWorldAndEarthAndPlantsAndAnimalsAndFoodChainAndBuildings(), followed by createEarthAndPlantsAndAnimalsAnd...(). – Delioth Dec 6 '16 at 18:28
  • @Delioth: No, you try to find a common term which describes the steps as a whole, like "createLife()". If you cannot find a common term, it is a hint that the function does not have a well-defined purpose. The use of "and" in a function name is an indicator of a bad abstraction. – JacquesB Dec 6 '16 at 18:58

protected by gnat Dec 7 '16 at 22:40

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.