211

I was trying to find alternatives to the use of global variable in some legacy code. But this question is not about the technical alternatives, I'm mainly concerned about the terminology.

The obvious solution is to pass a parameter into the function instead of using a global. In this legacy codebase that would mean that I have to change all functions in the long call chain between the point where the value will eventually be used and the function that receives the parameter first.

higherlevel(newParam)->level1(newParam)->level2(newParam)->level3(newParam)

where newParam was previously a global variable in my example, but it could have been a previously hardcoded value instead. The point is that now the value of newParam is obtained at higherlevel() and has to "travel" all the way to level3().

I was wondering if there was a name(s) for this kind of situation/pattern where you need to add a parameter to many functions that just "pass" the value unmodified.

Hopefully, using the proper terminology will allow me to find more resources about solutions for redesign and describe this situation to colleagues.

  • 94
    This is an improvement over using global variables. It makes it clear exactly what state each function depends on (and is one step on the road to pure functions). I've heard it called "threading" a parameter through, but I don't know how common this terminology is. – gardenhead Oct 31 '16 at 15:30
  • 8
    This is kind of too broad spectrum to have a specific answer. At this level, I'd call this just "coding". – Machado Oct 31 '16 at 15:32
  • 38
    I think the "problem" is just the simplicity. This is basically dependency injection. I guess there could be mechanisms that would automagically inject the dependency through the chain, if some more deeply nested member has it, without bloating the parameter lists of the functions. Maybe looking at dependency injection strategies with different levels of sophistication might lead to the terminology you are looking for, if there is one. – null Oct 31 '16 at 15:37
  • 7
    Although I appreciate the discussion about if it's a good pattern/antipattern/concept/solution, what I really wanted to know is if there is a name for it. – ecerulm Oct 31 '16 at 18:20
  • 3
    I've also heard it called threading most commonly, but also plumbing, as in lowering a plumb line throughout the call stack. – wchargin Oct 31 '16 at 19:07

10 Answers 10

202

The data itself is called "tramp data". It is a "code smell", indicating that one piece of code is communicating with another piece of code at a distance, through intermediaries.

  • Increases rigidity of code, especially in the call chain. You are much more constrained in how you refactor any method in the call chain.
  • Distributes knowledge about data/methods/architecture to places that don't care in the least about it. If you need to declare the data that is just passing through, and the declaration requires a new import, you have polluted the name space.

Refactoring to remove global variables is difficult, and tramp data is one method of doing so, and often the cheapest way. It does have its costs.

  • 73
    By searching for "tramp data" I was able to find "Code Complete" book on my Safari subscription. There is a section in the book called "Reasons to use global data" and one of the reasons is "Use of global data can eliminate tramp data". :) . I feel like "tramp data" will allow me to find more literature about dealing with globals. Thanks! – ecerulm Oct 31 '16 at 16:49
  • 9
    @JimmyJames, those functions do stuff of course. Just not with that specific new parameter that previously was just a global. – ecerulm Oct 31 '16 at 20:20
  • 177
    In 20 years of programming, I've literally never heard this term before nor would it have been immediately obvious what it meant. I'm not complaining about the answer, just suggesting that the term isn't that broadly used/known. Maybe it's just me. – Derek Elkins Nov 1 '16 at 4:04
  • 7
    Some global data is fine. Instead of calling it "global data", you can call it "environment" - because that's what it is. Environment might include, for example, a string path for appdata (on windows), or in my current project the entire set of GDI+ brushes, pens, fonts and so on, used by all components. – Robinson Nov 1 '16 at 10:39
  • 7
    @Robinson Not quite. For example, do you really want your image writing code to touch %AppData%, or would you rather have it take an argument for where to write? That's the difference between global state and an argument. "Environment" can just as easily be an injected dependency, only present for those who are responsible for interacting with the environment. GDI+ brushes etc. are more reasonable, but that's really more a case of resource management in an environment that can't do it for you - pretty much a deficiency of the underlying APIs and/or your language/libraries/runtime. – Luaan Nov 1 '16 at 13:06
102

I don't think this, in itself, is an anti-pattern. I think the problem is that you are thinking of the functions as a chain when really you should think of each one as an independent black box (NOTE: recursive methods are a notable exception to this advice.)

For example, let's say I need to calculate the number of days between two calendar dates so I create a function:

int daysBetween(Day a, Day b)

In order to do this, I then create a new function:

int daysSinceEpoch(Day day)

Then my first function becomes simply:

int daysBetween(Day a, Day b)
{
    return daysSinceEpoch(b) - daysSinceEpoch(a);
}

There's nothing anti-pattern about this. The parameters of the daysBetween method are being passed off to another method and are never otherwise referenced in the method but they are still needed for that method to do what it needs to do.

What I would recommend is looking at each function and start with a couple of questions:

  • Does this function have a clear and focused goal or is it a "do some things" method? Usually the name of the function helps here and if there is stuff in it that is not described by the name, that's a red flag.
  • Are there too many parameters? Sometimes a method can legitimately need lots of input but having so many parameters makes it burdensome to use or understand.

If you are looking at a jumble of code without a single purpose bundled into a method, you should start by unraveling that. This can be tedious. Start with the easiest things to pull out and move into a separate method and repeat until you have something coherent.

If you just have too many parameters, consider Method to Object refactoring.

  • 2
    Well, I didn't mean to be controversial with the (anti-) . But I still wonder if there is a name for the "situation" of having to update many function signatures. I guess is more of a "code smell" than an antipattern. It tells me that there is something to be fixed in this legacy code, if I have to update 6 functions signature to accomodate the elimination of a global. But I do think that passing parameters is ok usually, and I appreciate the advide on how to tackle the underlying problem. – ecerulm Oct 31 '16 at 15:28
  • 4
    @ecerulm I'm not aware of one but I will say that my experience tells me that converting the globals to parameters is absolutely the right way to start eliminating them. This eliminates shared state so that you can further refactor. I'm guessing there are more issues with this code but there's not enough in your description to know what they are. – JimmyJames Oct 31 '16 at 15:31
  • 2
    I usually follow that approach too, and probably will do in this case too. I just wanted to improve my vocabulary/terminology around this so I can research more into this and do better , more focused questions in the future. – ecerulm Oct 31 '16 at 15:35
  • 3
    @ecerulm I don't think there is one name for this. It's like a symptom that's common to a lot of diseases as well as non disease conditions e.g. 'dry mouth'. If you flesh out the description of the structure of the code, it might point to something specific. – JimmyJames Oct 31 '16 at 15:39
  • @ecerulm It tells you there is something to be fixed - now it's much more obvious that something is to be fixed than it was when it was a global variable instead. – user253751 Oct 31 '16 at 21:33
61

BobDalgleish has already noted that this (anti-)pattern is called "tramp data".

In my experience, the most common cause of excessive tramp data is having a bunch of linked state variables that should really be encapsulated in an object or a data structure. Sometimes, it may even be necessary to nest a bunch of objects to properly organize the data.

For a simple example, consider a game that has a customizable player character, with properties like playerName, playerEyeColor and so on. Of course, the player also has a physical position on the game map, and various other properties like, say, current and maximum health level, and so on.

In a first iteration of such a game, it might be a perfectly reasonable choice to make all these properties into global variables — after all, there's only one player, and almost everything in the game somehow involves the player. So your global state might contain variables like:

playerName = "Bob"
playerEyeColor = GREEN
playerXPosition = -8
playerYPosition = 136
playerHealth = 100
playerMaxHealth = 100

But at some point, you might find that you need to change this design, perhaps because you want to add a multiplayer mode into the game. As a first attempt, you could try making all those variables local, and passing them to functions that need them. However, you may then find that a particular action in your game might involve a function call chain like, say:

mainGameLoop()
 -> processInputEvent()
     -> doPlayerAction()
         -> movePlayer()
             -> checkCollision()
                 -> interactWithNPC()
                     -> interactWithShopkeeper()

...and the interactWithShopkeeper() function has the shopkeeper address the player by name, so you now suddenly need to pass playerName as tramp data through all those functions. And, of course, if the shopkeeper thinks that blue-eyed players are naïve, and will charge higher prices for them, then you'd need to pass playerEyeColor through the whole chain of functions, and so on.

The proper solution, in this case, is of course to define a player object that encapsulates the name, eye color, position, health and any other properties of the player character. That way, you only need to pass that single object around to all the functions that somehow involve the player.

Also, several of the functions above could be naturally made into methods of that player object, which would automatically give them access to the player's properties. In a way, this is just syntactic sugar, since calling a method on a object effectively passes the object instance as a hidden parameter to the method anyway, but it does make the code look clearer and more natural if used properly.

Of course, a typical game would have a lot more "global" state than just the player; for example, you'd almost certainly have some kind of a map on which the game takes place, and a list of non-player characters moving on the map, and maybe items placed on it, and so on. You could pass all those around as tramp objects too, but that would again clutter up your method arguments.

Instead, the solution is to have the objects store references to any other objects that they have permanent or temporary relationships with. So, for example, the player object (and probably any NPC objects too) probably should store a reference to the "game world" object, which would have a reference to the current level / map, so that a method like player.moveTo(x, y) does not need to be explicitly given the map as a parameter.

Similarly, if our player character had, say, a pet dog that followed them around, we would naturally group all the state variables describing the dog into a single object, and give the player object a reference to the dog (so that the player can, say, call the dog by name) and vice versa (so that the dog knows where the player is). And, of course, we'd probably want to make the player and the dog objects both subclasses of a more generic "actor" object, so that we can reuse the same code for, say, moving both around the map.

Ps. Even though I've used a game as an example, there are other kinds of programs where such issues come up as well. In my experience, though, the underlying problem tends to always be the same: you have a bunch of separate variables (whether local or global) that really want to be bunched together into one or more interlinked objects. Whether the "tramp data" intruding into your functions consists of "global" option settings or cached database queries or state vectors in a numerical simulation, the solution is invariably to identify the natural context that the data belongs to, and make that into an object (or whatever is the closest equivalent in your chosen language).

  • 1
    This answer provides some solutions to a class of issues that might exist. There might be situations where globals were used which would indicate a different solution. I take a issue with the idea that making the methods part of the player class is equivalent to passing objects to methods. This ignores polymorphism which isn't easily replicated in this way. For example, if i want to create different types of players that have different rules about movements and different types of properties, simply passing these objects to one method implementation will require a lot of conditional logic. – JimmyJames Nov 1 '16 at 15:27
  • 6
    @JimmyJames: Your point about polymorphism is a good one, and I thought about making it myself, but left it out to keep the answer from getting even longer. The point I was trying (perhaps poorly) to make was that, while purely in terms of data flow there's little difference between foo.method(bar, baz) and method(foo, bar, baz), there are other reasons (including polymorphism, encapsulation, locality, etc.) to prefer the former. – Ilmari Karonen Nov 1 '16 at 16:14
  • @IlmariKaronen: also the very obvious benefit that it future-proofs the function prototypes from any future changes/additions/deletions/refactorings in the objects (e.g. playerAge). This alone is invaluable. – smci Nov 1 '16 at 22:08
34

I am not aware of a specific name for this, but I guess it is worth to mention that the problem you describe is just the problem of finding the best compromise for the scope of such a parameter:

  • as a global variable, the scope is too big when the program reaches a certain size

  • as a purely local parameter, the scope might be too small, when it leads to lots of repetitive parameter lists in call chains

  • so as a trade-off, you can often make such a parameter a member variable in one or more classes, and that is what I would just call proper class design.

  • 10
    +1 for proper class design. This sounds like a classic problem waiting for an OO solution. – l0b0 Oct 31 '16 at 18:11
21

I believe the pattern you're describing is exactly dependency injection. Several commenters have argued that this is a pattern, not an anti-pattern, and I would tend to agree.

I also concur with @JimmyJames's answer, where he claims that it is good programming practice to treat each function as a black box that takes all its inputs as explicit parameters. That is, if you're writing a function that makes a peanut butter and jelly sandwich, you could write it as

Sandwich make_sandwich() {
    PeanutButter pb = get_peanut_butter();
    Jelly j = get_jelly();
    return pb + j;
}
extern PhysicalRefrigerator g_refrigerator;
PeanutButter get_peanut_butter() {
    return g_refrigerator.get("peanut butter");
}
Jelly get_jelly() {
    return g_refrigerator.get("jelly");
}

but it would be better practice to apply dependency injection and write it like this instead:

Sandwich make_sandwich(Refrigerator& r) {
    PeanutButter pb = get_peanut_butter(r);
    Jelly j = get_jelly(r);
    return pb + j;
}
PeanutButter get_peanut_butter(Refrigerator& r) {
    return r.get("peanut butter");
}
Jelly get_jelly(Refrigerator& r) {
    return r.get("jelly");
}

Now you have a function that clearly documents all its dependencies in its function signature, which is great for readability. After all, it is true that in order to make_sandwich you need access to a Refrigerator; so the old function signature was basically disingenuous by not taking the refrigerator as part of its inputs.

As a bonus, if you do your class hierarchy right, avoid slicing, and so on, you can even unit-test the make_sandwich function by passing in a MockRefrigerator! (You might need to unit-test it this way because your unit-test environment might not have access to any PhysicalRefrigerators.)

I do understand that not all uses of dependency injection require plumbing a similarly named parameter many levels down the call stack, so I'm not answering exactly the question you asked... but if you're looking for further reading on this subject, "dependency injection" is definitely a relevant keyword for you.

  • 10
    This is very clearly an antipattern. There is absolutely no call for a refrigerator to be passed. Now, passing a generic IngredientSource might work, but what if you get the bread form the breadbin, the tuna from the larder, the cheese from the fridge... by injecting a dependency upon the source of ingredients into the unrelated operation of forming those ingredients into a sandwich, you have violated separation of concerns and made a code smell. – Dewi Morgan Nov 1 '16 at 5:50
  • 8
    @DewiMorgan: Obviously you could refactor further to generalize the Refrigerator into an IngredientSource, or even generalize the notion of "sandwich" into template<typename... Fillings> StackedElementConstruction<Fillings...> make_sandwich(ElementSource&); that's called "generic programming" and it's reasonably powerful, but surely it's way more arcane than the OP really wants to get into right now. Feel free to open a new question about the proper level of abstraction for sandwich programs. ;) – Quuxplusone Nov 1 '16 at 7:49
  • 11
    Be there no mistake, the unprivileged user should not have access to make_sandwich(). – dotancohen Nov 1 '16 at 11:28
  • 2
    @Dewi - XKCD link – Gavin Lock Nov 1 '16 at 15:26
  • 19
    The most serious error in your code is that you are keeping peanut butter in the refrigerator. – Malvolio Nov 2 '16 at 3:36
15

This is pretty much the textbook definition of coupling, one module having a dependency that deeply affects another, and that creates a ripple effect when changed. The other comments and answers are correct that this is an improvement over the global, because the coupling is now more explicit and easier for the programmer to see, instead of subversive. That doesn't mean it shouldn't be fixed. You should be able to refactor to remove or reduce the coupling, although if it's been in there a while it can be painful.

  • 3
    If level3() needs newParam, that's coupling for sure, but somehow different parts of code have to communicate with each other. I would not necessarily call a function parameter bad coupling if that function make use of the parameter. I think the problematic aspect of the chain is the additional coupling introduced for level1() and level2() which have no use for newParam except to pass it on. Good answer, +1 for coupling. – null Oct 31 '16 at 16:41
  • 6
    @null If they really didn't have use for it they could make up a value instead of receiving from their caller. – Random832 Nov 1 '16 at 5:57
3

While this answer doesn't directly answer your question, I feel I would be remiss to let it pass without mentioning how to improve on it (since as you say, it may be an anti-pattern). I hope that you and other readers can get value from this additional commentary on how to avoid "tramp data" (as Bob Dalgleish so helpfully named it for us).

I agree with answers that suggest doing something more OO to avoid this problem. However, another way to also help reduce this passing of arguments deeply without just jumping to "just pass a class where you used to pass many arguments!" is to refactor so that some steps of your process occur at the higher level instead of the lower one. For example, here's some before code:

public void PerformReporting(StuffRepository repo, string desiredName) {
   var stuffs = repo.GetStuff(DateTime.Now());
   FilterAndReportStuff(stuffs, desiredName);
}

public void FilterAndReportStuff(IEnumerable<Stuff> stuffs, string desiredName) {
   var filter = CreateStuffFilter(FilterTypes.Name, desiredName);
   ReportStuff(stuffs.Filter(filter));
}

public void ReportStuff(IEnumerable<Stuff> stuffs) {
   stuffs.Report();
}

Note that this becomes even worse the more things that have to be done in ReportStuff. You might have to pass in the instance of the Reporter you want to use. And all sorts of dependencies that have to be handed along, function to nested function.

My suggestion is to pull all that to a higher level, where the knowledge of the steps requires lives in a single method instead of being spread across a chain of method calls. Of course it would be more complicated in real code, but this gives you an idea:

public void PerformReporting(StuffRepository repo, string desiredName) {
   var stuffs = repo.GetStuff(DateTime.Now());
   var filter = CreateStuffFilter(FilterTypes.Name, desiredName);
   var filteredStuffs = stuffs.Filter(filter)
   filteredStuffs.Report();
}

Notice that the big difference here is that you don't have to pass the dependencies through a long chain. Even if you flatten to not just one level, but a few levels deep, if those levels also achieve some "flattening" so that the process is seen as a series of steps at that level, you'll have made an improvement.

While this is still procedural and nothing has been turned into an object yet, it is a good step toward deciding what kind of encapsulation you can achieve through turning something into a class. The deeply-chained method calls in the before scenario hide the details of what is actually happening and can make the code very hard to understand. While you can overdo this and end up making the higher-level code know about things it shouldn't, or make a method that does too many things thus violating the single-responsibility principle, in general I have found that flattening things a bit aids in clarity and in making incremental change toward better code.

Note that while you're doing all this, you should consider testability. The chained method calls actually make unit testing harder because you don't have a good entry point and exit point in the assembly for the slice that you want to test. Notice that with this flattening, since your methods no longer take so many dependencies, they're easier to test, not requiring as many mocks!

I recently tried to add unit tests to a class (that I didn't write) that took something like 17 dependencies, all of which had to be mocked! I haven't gotten it all worked out yet, but I split the class into three classes, each dealing with one of the separate nouns it was concerned with, and got the dependency list down to 12 for the worst one and about 8 for the best one.

Testability will force you to write better code. You should be writing unit tests because you will find that it makes you think about your code differently and you'll write better code from the get-go, regardless of how few bugs you might have had before writing the unit tests.

2

You're not literally violating the Law of Demeter, but your problem is similar to that in some ways. Since the point of your question is to find resources, I suggest you read about Law of Demeter and see how much of that advice applies to your situation.

  • 1
    A bit weak on details, which probably explains the downvotes. Yet in spirit this answer is exactly spot on: OP should read up on the Law of Demeter — this is the relevant term. – Konrad Rudolph Oct 31 '16 at 23:14
  • 4
    FWIW, I don't think the Law of Demeter (a.k.a. "least privilege") is relevant at all. The OP's case is where his function would not be able to do its job if it didn't have the tramp data (because the next guy down the call stack needs it, because the next guy needs it, and so on). Least privilege / Law of Demeter is relevant only if the parameter is truly unused, and in that case the fix is obvious: remove the unused parameter! – Quuxplusone Nov 1 '16 at 1:22
  • 2
    This question's situation has exactly nothing to do with the Law of Demeter... There's a superficial similarity regarding the chain of method calls, but otherwise it's very different. – Eric King Nov 1 '16 at 15:04
  • @Quuxplusone Possible, though in this case the description is quite confusing because chained calls don’t really make sense in that scenario: they should be nested instead. – Konrad Rudolph Nov 2 '16 at 14:53
  • 1
    The problem is very similar to LoD violations, since the usual refactoring suggested to deal with LoD violations is to introduce tramp data. IMHO, this is a good starting point for reducing coupling but it is not sufficient. – Jørgen Fogh Nov 4 '16 at 11:53
1

There are instances where the best (in terms of efficiency, maintainability and ease of implementation) to have certain variables as global rather than the overhead of always passing everything around (say you have 15 or so variables that has to persist). So it makes sense to find programming language that supports scoping better (as C++'s private static variables) to alleviate the potential mess (of namespace and having things tampered). Of course this is just common knowledge.

But, the approach stated by the OP is very useful if one is doing Functional Programming.

0

There is no anti-pattern here at all, because the caller doesn't know about all these levels below and doesn't care.

Someone is calling higherLevel(params) and expects higherLevel to do its job. What higherLevel does with params is none of the callers business. higherLevel handles the problem in the best way it can, in this case by passing params to level1(params). That's absolutely Ok.

You see a call chain - but there is no call chain. There is a function at the top doing its job the best way it can. And there are other functions. Each function could be replaced at any time.

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