2

Consider this simple example:

def a(val_x, val_y):
    return 5 + b(val_x, val_y)

def b(val_x, val_y):
    return 1 + c(val_x, val_y)

def c(val_x, val_y):
    return val_x * val_y

Passing the arguments val_x, val_y from a to b to c seems like a common code smell (I know it is something I do all too often when prototyping code!) but I can't find a name for it.

Any suggestions?

closed as primarily opinion-based by Jörg W Mittag, gnat, Berin Loritsch, psr, Greg Burghardt Mar 30 '18 at 21:57

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    Given that a() is defined as a function of those values, how would you propose those values reach b() and c()? And would def halfsine(x): return 0.5 * sin(x) be considered equally-smelly because it passes x through to sin()? – Blrfl Mar 30 '18 at 17:29
  • Thanks Blrfl - I need to come up with a better example then what I have to highlight what I think is a problem. I also agree with what autophage has written so that means I'm not communicating well. – jobevers Mar 30 '18 at 17:49
  • What's the alternative? Increasing the scope of the variables so you don't have to pass them would be a much worse code smell. – John Wu Mar 31 '18 at 2:49
5

This architecture is the opposite of dependency inversion. I'm not aware of an official name, but you might call it uninverted dependencies. It's a high-level component depending on a lower-level component. When you invert the dependencies, you end up with something like:

def a(b_result):
  return 5 + b_result

def b(c_result):
  return 1 + c_result

def c(val_x, val_y):
  return val_x * val_y

And you call it like:

a(b(c(x, y)))
5

There's nothing wrong with this at all, though if you have a method that does nothing but call another method it might be worth seeing whether you can get rid of the intermediate method.

Is that the thing that you're implicitly trying to ask about?

If so - there are plenty of reasons you might want such an intermediate method. For example, maybe a is public but b is private. Or maybe b is a placeholder and will get more code added later.

[EDIT to address the more specific example]

No, this is totally fine and not an antipattern at all. Each method is just a named thing you want to do, and some of those things may require data from further up the chain of things you're trying to do.

Say you're trying to calculate the volume of a cylinder. The formula for that is (pi * radius^2) * height. You could define a class getCylinderVolume(radius, height) that just returns the result directly.

But say you decide that you want to extract the "multiply by the height" piece of that (say, because you want to warn the user that the thing whose volume they're calculating will exceed a legal height limit). You might then change getCylinderVolume's implementation to instead return getCircleArea(radius) * checkHeightAndBubbleErrorIfTooTall(height).

You might also realize that you need to account for interior and exterior volumes separately...

The point is that there are plenty of reasons you might want to keep passing a variable straight down a call stack. Doing so often means that you're doing a good job of keeping your functions small.

  • Thanks. I modified the example to clarify better the issue I'm asking about. – jobevers Mar 30 '18 at 16:33
1

I totally agree with autophage and want to present an alternative that I run into too often that actually is bad.

I can only guess why people do this but I think I understand why. They must be thinking something like

"Hey, I am going to need these variables in a number of methods, so I might as well declare them as class members. That way I can conveniently access them from any method. How DRY!"

This is what you should be wary of. On the class level this is sheer noise, these variables do not mean anything on the class level but any outsider/successor trying to get his head around the class does not know this so his head will hurt trying to understand what it all means. It will take him some time and energy to get to understand these are really just local variables or method arguments. And he will hate and loath you for leaving him this crap. And then refactor it, complaining to his co-workers about what an idiot you are.

The bad cases will have two or three data members that are real class members and ten or more handy-dandy variables obscuring them.

0

Extract an object. Collect x and y into a new Point class. Then move the functions a,b,c into the Point class.

  • Definitely a good refactoring! I was curious though if the problem has a name. – jobevers Mar 30 '18 at 16:54

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