loading...
Thinkster

Code Smell: Output Parameters

josepheames profile image Joe Eames ・3 min read

Output parameters are simple. They are parameters passed into a function or method call that are modified during the function.

Output parameters are the kind of code smell that is sometimes difficult to see, but when they crop up they can cause a moment of confusion, as they obscure the purpose of a function call, and therefore make our code less readable.

We spend WAY more time reading code than writing it, so we want our code as readable as possible.

Let's look at an example, and discuss how to address it.

image

Here we have a fairly simple algorithm. It's named appropriately. We are creating a report for a course. Maybe it's for Thinkster's new course on creating web API's with .NET Core (wink wink).

Look through what the function does, looking at each line. If we do that we see that the steps are pretty straightforward. First, we create a new report. Then we initialize it with some basic info. But then….then something is weird. If you looked at this you MIGHT have noticed that the next step is just a bit confusing.

image

The method name is clear - we are adding analytics. But the object being invoked, and the value passed in, they aren't quite so obviously telling us what is going on. We're calling "add analytics" on the course. And we're passing in the course report. The question is, who is being modified in this operation? Is the course getting modified? Are analytics being added to the course? Is the report being modified? Are analytics being added to the report? Are both of them being modified? We'd have to drill into the method to understand even this basic piece of information?

This is the problem. When reading the algorithm at this level of abstraction, we don't want to have to drill down into a method to understand the basics of the operation. Sure if we want to know exactly what analytics are added, and how they are added, then we want to drill down. But when we just want to know the major steps for creating a course report, we want to be able to look at the method, and NOT have to drill down. Ultimately the more code we have to read, the more difficult our job.

There are several problems with having to read extra code to understand what's going on. First, it takes more time and involves more navigating around. Second, it increases our cognitive load. We have to keep more details in the "working RAM" in our head. We have to juggle more and more pieces of information. This not only makes it more likely that we'll forget an important detail and add a bug, but it also makes programming less enjoyable and increases burnout. Ultimately the fewer things we have to keep in our brain at one time, the more effectively we can write and maintain our code.

In the case of this code smell, the problem is that what's actually going on here is that the item being modified is the course report, which is a parameter. In general, functions shouldn't modify parameters. When invoking a function, the items being modified should be obvious. It should either be the object whose method we call, OR the return value of the call is applied somewhere. This problem can also occur in more functional programming, and there it's just as problematic. Good functional programming discourages this practice just like good OO programming does.

So how do we solve the issue? Here's a potential fix to the code above:

image

Here we make it far more explicit as to what's going on. The courseReport is being modified. The course itself doesn't appear to be getting modified according to what we see here.

A good way to sum this up is in the following advice: Functions should either return a value or modify "this". No other mutations should be allowed. And if we're doing true functional programming, then we only return values, no modifications of "this".

Happy Coding!

Enjoy this discussion? Sign up for our newsletter here.

Visit Us: thinkster.io | Facebook: @gothinkster | Twitter: @gothinkster

Discussion

pic
Editor guide
Collapse
joeyhub profile image
Joey Hernández

I'd not change the direction of the edge on a model purely for minor aesthetics.

However, reducing a two way relationship to a one way relationship tends to simplify a object graph.

What's wrong with new CourseReport(course, date)?

currentDate seems to appear from the aether.