Suppose I have the following
Class A {
Foo getFoo();
Bar getBar();
Baz getBaz();
}
And I need to define a function doStuff that uses Foo, Bar, Baz of one object and does some stuff
I'm struggling between which method of implementing doStuff is better (suppose it would be undesirable to place doStuff inside class A)
Method A
void doStuff(Foo foo, Bar bar, Baz baz)
{
//some operation
}
or
Method B
void doStuff(A a)
{
Foo foo = a.getFoo();
Bar bar = a.getBar();
Baz baz = a.getBaz();
//some operation
}
To my limited knowledge,
(+ pros, - cons)
Method A
+It is clear exactly what parameters doStuff() operates on
-Susceptible to long parameter lists and more susceptible to user mistakes
Method B
+Simple, easy to use method
+Seems more extensible (?)
-Creates unnecessary dependency towards class A
Can anyone share additional insight towards the pros and cons of these two methods?
Method A (naked parameters) always has the advantages that
- it requires the method author to type less, since they don't have to implement a Parameter Object,
- it requires the method caller to type less, since they don't have to instantiate a Parameter Object
- it performs better, since no Parameter Object has to be constructed and garbage collected
- the reader can see what the individual parameters are from the method signature alone (but this is a double-edged sword; see below)
Method B (Parameter Object) has advantages when
- the parameters have domain meaning as a group, so the Parameter Object can be given a name that explains that meaning, saving the reader from having to read and understand each member of the group and how they relate
- the parameter list is used in more than one method, so using the Parameter Object in each reduces duplication
- the values in the parameter list are passed around among multiple methods as a group, which is easier when they can be passed as a single Parameter Object
- some combinations of values are invalid; the Parameter Object can prevent those combinations
- some values are optional, which can be provided by the Parameter Object instead of (depending on your language) default parameter values or overloaded methods
- there is more than one parameter of the same type, making value-swapping errors more likely (although a Parameter Object is not better in this case if it
has a constructor with the same parameter list as the method)
That the Parameter Object introduces a new dependency on which caller and callee depend is not much of a disadvantage, since it is a simple class with no dependencies of its own.
So, Parameter Object is
- almost never worthwhile for a single parameter, sometimes worthwhile for a two-parameter method (e.g. Point is usually better than x, y) and sometimes not, and increasingly helpful with three and more parameters
- increasingly helpful when more methods use the same parameter list
Parameter Objects do provide a nice approach to encapsulate related parameters to reduce the total parameter count to any method or constructor.
One should be very careful to make sure that the parameter objects do actually contain truly related parameters.
Actually there are multiple ways of approaching this problem depending on the parameter types you are dealing with.
If you are dealing with parameters that are general types like more than one Strings or Ints and there is a potential for a client to actually pass in the wrong sequence of arguments ,it often makes more sense to create custom types ie. create enum with possible values. That can provide good compile time check for your arguments.
Another good use of them is you can use them to return complex values from functions. See here.
Another approach i take a lot of times is to check and see if the work done by the doStuff method be broken down into simpler methods with less dependencies.
Principally i try to follow Bob Martin's recommendation of a maximum of three parameters. Well he actually say it should be mostly not more than one !
Any increase should have justified reasons.
Refer this excellent book : Clean Code
Both David and Som's answers have great information to consider. I'll add the following:
As with many design patterns, the decision of what to do lies on a continuum between options with their own pros and cons. There is not always a right answer - more often it comes down to which pros to you want to enjoy and which cons are you willing to risk.
In my experience, moving to a DTO is helpful when you have related values that always travel together. David described the pros to this approach well. An additional con I have seen to this approach is that you risk adding unnecessary dependencies to methods when the DTO grows.
For example, methods A, B, C, and D take Foo, Bar, and Baz, so it is nice to combine these arguments in to a DTO. Then methods A and B need to take on Quux - do you add Quux to the DTO forcing C and D to take on an unused dependency? When you test C and D, what value do you pass in for Quux? When a new developer uses methods C and D, does the presence of Quux generate confusion? When comparing methods A and C, is it clear how Quux should or should not be defined?
Similar situations arise when you originally need Foo, Bar, and Baz for all methods, but then some methods no longer need those values.
I observed an experience where one team was passing a DTO to the service of another team and taking great efforts to correctly populate and sync the information in that DTO, when all that was actually needed was a single value that could have been passed trivially.
Unless the values always go together, you risk generating confusion, increased test burden, and extra dev work. If the values do always go together, a DTO can provide clarity, reduce duplication, simplify consistency, etc.
Consider a Customer that has an Address and an CurrentInvoice. Which is more correct -
SendInvoiceToAddress(Invoice invoice, Address adress);
or
SendInvoiceToAddress(Customer customer);
I think both. Or stated differently - that really depends on your app.
If every invoice belongs (by definition) to a single customer, that's one thing. and that would imply that your method exists inside CustomerInvoiceSender class (or something like that). This is something that lies entirely within the customer domain. Each month you want to send that months' invoice (and nothing else).
If you want to send multiple invoices to multiple adressess (not neccesarily for customers, for any purpose), well than that's a completely different story. It would probably also exist inside InvoiceSender class (or something like that). It also has nothing to do with the Customer domain. Customers in this case is just one small case of Invoices being shipped. Also worth noting is that in this case you might want interfaces instead of concretes becuase a customer's invoice and say a company's invoice might be two very different classes that just happen to share a common interface ("a couple of properties" in this case).
