javascript:S107 objection

Good evening!

Let me start with the description offered:

“A long parameter list can indicate a new structure should be created to wrap the numerous parameters or that the function is doing too many things.”

I normally appreciate the hell out of this rule, but this evening it’s a non-idiomatic thorn in my side. Aggravated by the fact that the rule was provoked by going along with the spirit of its suggestion.

The exemptions provided are both not JavaScript, but derived languages that closely resemble JavaScript. Those are not acceptable alternatives in the context I’m working.

So, what is it I was trying to do? Create a class that would be a structure that replaces way too many parameters. So, how would you go about initializing that class?

Typically, the idiomatic way is with a constructor:

class DataThing {
   constructor(a, b, c, d, e, f, g, h, I) {
      this.a = a;
      //…
      this.i = i;
      }
   }

It’s a little more complex than that, in that data cleaning and validation is also being done at the time the object is being initialized.

So, what are the alternatives?

I see a few:

  • Define the properties, but initialize the class using a long list of assignments. This doesn’t reduce cognitive load, but merely displaces it. Furthermore, it moves the onus of cleaning and validating the data out of the class. THIS IS A SEVERE CODE SMELL.
  • Define the properties, and define setters that do the data cleaning, then set all the properties outside of the class using the setter methods. This is improved, but also honestly fails to reduce cognitive load. It merely displaces it. THIS IS STILL STINKY.
  • Define the parameters in a dictionary, and pass the dictionary to the constructor. This can still allow us to put the validation logic in the constructor, but holy HELL does this version stink. We are, at best, creating an anonymous object to pass to a constructor. Meaning we’re making an object just to create an object. By my count, we’re now doing assign/get/assign for every parameter instead of merely an assign.
  • Define the parameters in a custom object and pass that object to the constructor. CONGRATULATIONS, we have recreated the original problem, but now it’s a sub-problem of the original problem. Rise, recurse, repeat.
  • Define smaller specialized classes, instead, that don’t encapsulate the full tuple. This will pass SonarLint’s sniffer but the idea of passing around two or three different objects when it could have been just one — or, and this is bizarre — use those smaller classes as parameters to the constructor of the originally desired class. Both of these are phenomenally bad code smells, with the latter deserving special mention for bonus awkwardness.

My question is this:

Can we discuss what IS possibly an acceptable, idiomatic solution in JavaScript. ONLY JavaScript. Is there an alternative factoring I’m missing? Because, to my nose, in this context, I’m going to have to create a garbage factoring that smells worse than what I originally wrote.(Probably the second option, though it’s far more verbose than I want it at least is the least-awful option that will get me past the gate.

But it’s looking to me like constructors need to not be called out by this rule, even if we need to define some additional guard rails for the exception so that the intent of flagging something that deserves it.

Hi Talia,

Sorry for the huge delay. Not sure if you’re still interested in this.

I think you can agree that having too many arguments is not great code.
And you’re perhaps asking about some ideas about what potential solutions we have in JS, not TypeScript or something else.

In this sense, I don’t think we are talking about a False Positive here. The rule is doing its work of pointing out not-so-great code.

As to how would one go about improving such a class, well, of course, it depends.
I can share my own thoughts.

I would say too many arguments is a symptom, but the root issue could be something like:

1/ The class interface is poorly defined, it fails to encapsulate its dependencies or properly define data abstractions → In this case, consider if some of the arguments are highly related and could become either a data object or a class themselves.

2/ The class is actually much more than “one thing”, it’s doing way too much and knows way too much → Consider splitting it, and if you still need a “single” entry point you can use a mixin (we have class expressions in JS after all), a façade (either statically or with a Proxy), a factory (boring Java kind of code), or something.

I think the rule description is trying to point in these directions. Maybe it can use some improvement. There might be other causes, and thus other solutions. But it’s fair to say these are the most common.

Happy to have a deeper discussion if you can provide more context.