Nothing Special   »   [go: up one dir, main page]

DEV Community

Cover image for Code Smell — Too Many Parameters
Joe Eames for Thinkster

Posted on

Code Smell — Too Many Parameters

Although the "Long Method" or "Large Class" code smells are often the easiest ones to spot, one of my favorite code smells to investigate is the "Too Many Parameters" code smell.

Too Many Parameters is a code smell that is indicated by a method or function that takes in a very large list of parameters. Imagine a function that takes in twenty parameters. (as an aside, if you aren't clear on the difference between arguments and parameters, check out this very succinct answer) Twenty parameters is a crazy large amount. Something is obviously wrong here.

Let's look at a concrete example. Here we have an example method that creates a schedule from a list of classes:

image

There are obviously some problems here. What happens when there are more than 10 classes? What happens when there's less? Imagine what this code must look like. There's an obvious answer here to change this list of parameters into a single parameter that is a collection.

But here, the fix is not necessarily what we are concerned with. Instead, we are focusing on the fact that this is a code smell. This likely indicates a bigger problem in our code. If we were to see this, then looking at the implementation code, we are likely to find that there is some rather difficult-to-maintain code. THAT is what a code smell is. Something that leads us to look deeper, find out if there truly is an issue. It's hard to imagine that the above method can't be improved by changing it to this:

image

But not every code smell indicates a problem. They just indicate the potential for a problem.

Now you may say to yourself "The above isn't realistic. Nobody would write code like that." But look at the following method:

image

Now imagine what happens when we find out that we have a single class that actually has two different instructors. So we make the following simple change:

image

This is a LOT more realistic. Stuff like this happens all the time. Over time, the parameter list grows. This is how we end up with something like the first example.

Let's look at a more likely example:

image

Here we have a method that calculates the tuition for a student. Now we have eight parameters. Just looking at this method signature should give us pause. There's a lot of different information being passed in. Using the "Too Many Parameters" code smell, our nose wrinkles when we see this. And we look at the code. Maybe we find that the method is actually doing too much. Why is the student name and ID in this method? What does that have to do with the function of calculating tuition? Can a student's tuition vary based on their name? Or is the method doing too much? The last two parameters - isResident and isLegacy - are some kind of flag. Undoubtedly we'll find if statements or switch statements based on them. We may find a long method with a lot of branches that are difficult to understand, and therefore difficult to maintain.

Let's look at one more example:

image

Here we have a method that prints out a student's schedule. There are only six parameters. Is that too many? Certainly, some developers would say no. But if you have an eye for code smells, and a low tolerance for long parameter lists, you may say "anything over four is a code smell" and so when you see six, you look deeper. In this case this parameter list may indicate a problem in the code. Look at the first three parameters: the student's first name, last name, and ID. They are all related. If we need those three pieces of information about the student, it could be likely that we will soon need another piece of information, and then end up adding another parameter. Maybe we started with just the student ID, then later added the last name, and then finally in a different change, we added the first name. Each time it was natural to just add one more parameter. Our code slowly grew from a beautifully crafted piece of art into a hodgepodge of changes.

Maybe when we investigate the implementation we don't find a problem. But if we investigate when we see these code smells, we may find trouble brewing.

Code smells are often related to each other. Too Many Parameters can often be seen next to "Long Method" or "Large Class" and often indicate a violation of the Single Responsibility Principle. A keen "nose" helps us keep our application from growing into an unmaintainable mess. Contrary to some funny Dilbert cartoons, coding up a mess of spaghetti code that nobody else can decipher is NOT job security. Writing code that is easy to change is the best way to job security. Over the long term, team velocity is determined far more by the ongoing health of the code base than by any other factor.

Happy coding!

Signup for my newsletter here.

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

Top comments (7)

Collapse
 
kallmanation profile image
Nathan Kallman

Reminds me of one of the first codebases I worked in. So many function calls that looked like this:

doItAll(sensibleArgument, false, false, false, false, false, true)

Great article!

Collapse
 
itachiuchiha profile image
Itachi Uchiha • Edited

In different languages, you can use the same way. Let me show you a few examples.

This is an example for C#

class Student
{
    public int Age { get; set; }
    public string Name { get; set; }
}

public void SaveStudent(Student student)
{
    Console.WriteLine($"{student.Age} and {student.Name}");
}

And it should be written in Python like that;

class Student:
    def __init__(self):
        self.Age = None
        self.Name = None


def save_student(student: Student):
    print(f"Age {student.Age} and Name {student.Name}")


student = Student()

student.Age = 27
student.Name = "Ali"

save_student(student)
Collapse
 
patarapolw profile image
Pacharapol Withayasakpunt • Edited

Because of JavaScript, I feel that one Object parameter might be preferred, rather than multiple, or varargs.

It makes ordering of params not to matter.

Collapse
 
pentacular profile image
pentacular

I think you may have missed the point of the post. :)

Collapse
 
habereder profile image
Raphael Habereder • Edited

How so?

If I see parameters like studentID, studentName, studentAge, etc. all of these could be grabbed from a single Student object. Even dependencies like classes and fees should be retrievable by the same object via references.

A single object as parameter, where applicable, makes the whole function easier to structure

Collapse
 
macdoggie78 profile image
Rogier van Doggenaar

What is you have a class that merely serves as a payload or a DTO, and de payload is supposed to have a lot of data fields in there. If the data is hierarchical you can combine stuff in objects that you pass, but all flat data we just add as arguments to the constructor of the object. Sonar cloud now complaints that we have too many arguments in those objects.

The object is not doing too many things as it is only serializing the data.

Should we just ignore those cases?

Collapse
 
mcsee profile image
Maxi Contieri

sad but true