0

I often end up with methods with long number of arguments (up to 5 arguments). for ex -

public void doSomething(obj1, obj2, obj3, obj4, obj5)

obj1 to obj5 are unrelated to each other

I thought of creating a Bigger Object obj containing obj1 to obj5 then pass only obj object that is -

public void doSomething(obj)

But obj is only a convenient object here and does not have to have obj1 to obj5 as it member variables. Is it ok to use such convenient object or is there any other approach that I can leverage?

Updates

To answer about SRP, method doSomething calls some other methods which are responsible for their operation. Hence doSomething looks as -

public void doSomething(obj1, obj2, obj3, obj4, obj5) {
  doSomethin1(obj1)
 .doSomethin2(obj2)
 .doSomethin3(obj3)
 .doSomethin4(obj4)
 .doSomethin5(obj5) 
}

I was chaining and calling of these methods individually in tests but that seems code repetition to me and decided to create one method instead which takes all of the required arguments. But with this approach I was perplexed with having to pass to arguments to doSomething method

8
  • 4
    If they are so compltely unrelated, why are they both needed for that function? Obviously, there is some relationship between them.
    – Polygnome
    Sep 28 '17 at 8:40
  • @Polygnome they seem unrelated to me, please check my linked comment - softwareengineering.stackexchange.com/questions/358231/…
    – Tarun
    Sep 28 '17 at 9:00
  • Yeah, and your comment show that they are, in fact, related, at least to some degree. And if some objets truly are unrelated, they have no business in the same method, because thats a sign your method violates the SRP.
    – Polygnome
    Sep 28 '17 at 9:19
  • If you often find yourself in this situation, then I think you are doing something wrong. It seems less important to consider ways of dealing with this problem than to consider why you are having this problem in the first place. Sep 28 '17 at 9:22
5

I'd recommend to create an object class only in cases where the aggregation of the elements represents a thing that you can name, thus implying a useful name for the new class.

As you say the arguments are unrelated, I'd not go for a aggregate class.

I don't think it improves readability, comparing the two versions:

// not nice, but OK
doSomething(a, b, c, d, e);

// only makes things more complicated
DoSomethingArgs args = new DoSomethingArgs(a, b, c, d, e);
doSomething(args);

But having a method with so many arguments might imply that the method does too many things and could be refactored to smaller, more focussed parts, needing less arguments (remember the single-responsibility principle?).

1
  • I updated my question on SRP and my dilemma
    – Tarun
    Sep 29 '17 at 13:26
2

Are obj1 to obj5 really "unrelated to each other" when this method needs them all together?

If you consider this for a specific case, then I think that you will very often realize that those objects are, in fact, conceptually related. Maybe not all of them, maybe only three of the five.

But then, the "convenienve object" to hold those related objects together will have a meaning, and will be the natural place for some methods to deal with those related objects.

In fact, you might end up moving your doSomething() into that class, and calling

objectContaintingObj1ToObj3.doSomething(obj4, obj5)
1
  • For ex I need to fill two step registration form using selenium. To be able to fill this form I need couple of objects, for ex - Region (aka which country registration is for), Distributor (aka type to customer to register) WebDriver object, Test Data objects for registration step1 and step2 . Test data changes depending on Distributor. I had written all of them in different methods hence each method takes only one Object. But then I kept repeating and chaining these methods on different tests and wanted to write one method combining all of these steps. And hence the questions.
    – Tarun
    Sep 28 '17 at 9:00
1

The question is pretty abstract. There is a code smell in the sheer count of parameters, but it doesn't mean it's wrong code.

It also doesn't mean that the extreme other direction — make all the parameters into a single parameter object — is the right answer (though it could be).

So, the answer I offer is that while you say the objects are unrelated to each other there is potentially a meaningful abstraction for the binding of two or more of these together — even though individually they are unrelated.

Note that I'm looking at this from the point of view of a consuming client programmer rather than as an implementation detail of the method's signature.

If there are similar calls in the vicinity of this call:

doSomething(obj1, obj2, obj3, obj4, obj5)

such as:

doSomethingElse(obj1, obj3, obj6)

This hints there is an identifiable abstraction that comes from binding of of obj1 & obj3 together, and, perhaps this abstraction should be made manifest.

What we're trying to do is to reduce the burden on the client who is the consuming programmer (perhaps you). The fewer items the programmer has to deal with, the better, and less error prone. So, if you can bind some of these together into an abstraction that will reduce the number of items the consumer has to deal with.

However, introducing another abstraction (e.g. to hold all the parameters), especially if this abstraction would only be used in one place, doesn't help toward this goal. You should keep an eye out for a meaningful abstraction (especially one usable more than once) but if you don't find it then that's that..

0

I thought of creating a Bigger Object obj containing obj1 to obj5 then pass only obj object

Instead of doing this, considerd the Replace Method with Method Object refactoring. Create a class that has these (final) properties and create the doSomething method there. If you have other methods that take the same parameters or a subset of them, those can go there too. Private nested classes are a good option here if this is all internal to the current class.

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