The problem
What is the difference between a param object with an optional field which has a "field not set" value, and a method with explicit parameters for which null is a valid value to represent "optional parameter not set", and why is one acceptable and the other not?
Let's say you have an UpdatePerson
use case. Person
has many properties, some of which are Address
and Employment
(for the example code below, I'm going to only list these two properties for brevity's sake). You want to be able to call UpdatePerson
, but only provide partial information about the person (i.e. only the fields that need actual updating). Any missing parameters should be ignored and thus remain unchanged.
Therefore, you create your method:
public void UpdatePerson(int personId, Address address, Employment employment)
{
// ...
}
This example touches on the spirit of your approach with many optional parameters.
So how do you call this method when you want to update the person's address, but not their employment? Well, as you said:
a method with explicit parameters for which null is a valid value to represent "optional parameter not set"
Therefore, you want to do something like:
myPersonService.UpdatePerson(123, newAddress, null);
Okay, that works.
Second question, what if this person gets a new job, but does not move house? Well, similarly:
myPersonService.UpdatePerson(123, null, newEmployment);
So far, so good.
Third question. How do you call this method when the person loses their job? Therefore, their employment should be nullified (hint hint). Which leads to:
myPersonService.UpdatePerson(123, null, null);
But by applying your approach from above, how do we distinguish between not updating the address because it is null, and nullifying the employment because it is null?
When null
is a meaningful value in and of itself, null
cannot at the same time be used to indicate the absence of a value.
JacquesB's answer is also correct that you may want to reserve null
specifically to indicate a bug, therefore also giving you a reason to not give null
an intended meaning. It's the same as what I described in principle, just a different concrete example.
The problem here stems from you using the employment
method parameter in two ways: as the value holder for the person's Employment
property, and as an indicator for whether the UpdatePerson
method should update the person's employment or not. Your meanings for null
clash, and it becomes impossible to figure out which of the two meanings you're conveying.
Tangentially:
When IndexOf
cannot find an occurrence, it returns -1
. The reason it is able to do so is because -1 is a meaningless index value, and therefore it can be given a special meaning unrelated to the "normal" meaning of the returned integer value. If -1
were a meaningful index value, which in some languages it is (negative index = counting from the back), then IndexOf
wouldn't be able to use it as a special "nothing found" value.
The solution?
You might think to yourself that you can therefore use null
as a way to indicate optional parameters if null
is not a valid value to be used (for Person.Employment
). Technically, you could implement that. But it's a bad idea, for several reasons:
- This requires a reader to have perfect knowledge of all accepted values for all properties at all times, before the reader can understand exactly what is happening when you call
UpdatePerson(123, null, null)
. Which of these is being updated? Which isn't?
- You may change the implementation in the future, suddenly (dis)allowing
null
to be used as a valid value in the entity. This would then force you to have to re-evaluate all usages of this method, in order to now start/stop using null
to indicate the absence of an optional parameter
- You have to use two different systems: one for properties that can be
null
and one for properties that can't be null
.
It is much easier to steer away from using null
, and instead using a different marker for presence/absence of a parameter. For example, you could wrap your values in a wrapper class that explicitly indicates absence/presence.
Forgive the C# syntax, I'm no Java dev. As far as I'm aware, I'm only using language features that also exist in Java.
public class Updatable<T>
{
public Updatable(T value)
{
this.Value = value;
this.ShouldBeUpdated = true;
}
public Updatable()
{
this.ShouldBeUpdated = false;
}
public T Value { get; }
public bool ShouldBeUpdated{ get; }
}
Now you have a way to see the difference:
// Sets a new value
var setToNewValue = new Updatable<Employment>(new Employment());
// Sets to null
var setToNull = new Updatable<Employment>(null);
// Does not update, retains old value
var ignore = new Updatable<Employment>();
Which means your method can meaningfully understand what needs to be done:
public void UpdatePerson(int personId, Updatable<Address> address, Updatable<Employment> employment)
{
var person = Get(personId);
if(address.ShouldBeUpdated)
person.Address = address.Value;
if(employment.ShouldBeUpdated)
person.Employment= employment.Value;
Update(person);
}
This is just a basic solution, but it covers the biggest issue. Note that while I didn't particularly wrap your updatable properties in a single DTO, it's definitely worth considering doing so. I just kept it simpler for my example's purpose.
Clean coding advice
You've been given this feedback before, both in this question and the previous one, but the problem here is bigger than just what your question focuses on.
Using a silly example, if you ask "should I hold my breath when jumping in a pool of lava?", while technically not an answer to the literal question you're asking, the most sensical answer is "don't jump in a pool of lava to begin with".
Similarly, having too many method parameters is sign of a bad method design. Rather than finding ways to accommodate this design, it should be re-evaluated in its entirety so you never get to this problem area to begin with.
However, you asked your question with a very narrow scope, and it would require a broader view on the situation to find out how you should restructure your classes/methods in order to not end up with such a vast collection of optional parameters.