Java – Recommended Value Instead of String Parameter

clean codejavamethod-overloading

We have a method called attachDevice(Device device) which has only one argument. We had a situation to overload this method with one more parameter as like attachDevice(Device device, String deviceName).

with single argument

public void attachDevice(Device device)
{
    ..
    ..        
}

with double arguments

public void attachDevice(Device device, String deviceName)
{
    ..
    device.setName(deviceName);
    genericDeviceMap.put(deviceName, device);
    ..
}

Actually, my Team Lead asked me to make these two methods into a single generic call. The overloaded method only have two additional lines than the single method (which are shown as above). I can pass empty string instead of passing the deviceName, because the invocation of overloaded method will be very lesser than the invocation single argument call. But how bad it is that if passing null value since i don't set the name if the argument will be null. Which would be the best practice for this scenario?

Any suggestions are highly appreciated.

Note: The actual problem I stated here is, that

i) I wanted to merge overloaded methods into one method which should be generic to avoid code duplication

ii) Need a recommended value to pass to the method (either empty string or null value) to stick with best coding practices.

From the given answers, I got the best solution for my problem. The linked question which has been referred this question duplicate also provides info about handling null value, but not covered the first point I mentioned in the note. And I didn't write bad code by passing null as parameter, since I am aware that NPE will thrown sometimes.

Thanks.

Best Answer

I would discourage you to ever use null since it can lead to a further NPE, which are hard to debug (and cost a lot if they occur in production code).

Solution 1 (overload method)

If no deviceName is provided, you can provide a default one instead. The biggest disadvantage from this approach is the danger in genericDeviceMap.put(deviceName, device) because it can silently override the entry whose key is the default name (therefore, losing track of the previous Device).

public void attachDevice(Device device)
{
    attachDevice(device, "DefaultName");       
}

public void attachDevice(Device device, String deviceName)
{
    ..
    device.setName(deviceName);
    genericDeviceMap.put(deviceName, device);
    ..
}

Solution 2 (extract method)

Maybe that with your current architecture it doesn't make sense to add an entry to genericDeviceMap when attachDevice is called without a name. If so, a good approach is to only extract the common behaviour between the two attachDevice into private methods. I personnally don't like this approach for 2 reasons:

  • The behaviour between the two attachDevice is not the same, one has a side-effect (device.setName(deviceName)) and the other not
  • The side-effect in itself who often lead to subtle bugs because you alter an object who's coming from an outside scope

Code:

public void attachDevice(Device device)
{
    preAttachDevice();
    postAttachDevice();      
}

public void attachDevice(Device device, String deviceName)
{
    preAttachDevice();
    device.setName(deviceName);
    genericDeviceMap.put(deviceName, device);
    postAttachDevice();
}

private void preAttachDevice()
{
    ...
}

private void postAttachDevice()
{
    ...
}

Solution 3 (remove method)

My favorite, but the hardest. Ask yourself if you really need these two methods ? Does it make really sense to be able to call attachDevice either with a name or not ? Shouldn't you be able to say that attachDevice must be called with a name ?

In this case the code is simplified to only one method

public void attachDevice(Device device, String deviceName)
{
    ..
    device.setName(deviceName);
    genericDeviceMap.put(deviceName, device);
    ..
}

Or on the other hand, do you really need to maintain a Map of devices and devices names and set the device's name ? If not, you can get rid of the second method and only keep the first one.

public void attachDevice(Device device)
{
    ...
    ...     
}
Related Topic