is this way of calling a function a bad practice?

Question

I have the following code:

public void moveCameraTo(Location location){
    moveCameraTo(location.getLatitude(), location.getLongitude());
}

public void moveCameraTo(double latitude, double longitude){
    LatLng latLng = new LatLng(latitude, longitude);
    moveCameraTo(latLng);
}

public void moveCameraTo(LatLng latLng){
    GoogleMap googleMap =  getGoogleMap();
    cameraUpdate = CameraUpdateFactory.newLatLngZoom(latLng, INITIAL_MAP_ZOOM_LEVEL);
    googleMap.moveCamera(cameraUpdate);
}

I think that with this way I eliminate the responsibility of knowing what is a LatLng in another class, for example.

And you do not need to prepare the data before calling the function.

What do you think?

Does this approach have a name? Is it really a bad practice?

Answer 1

You are using your languages method overloading feature to offer the caller alternate ways of resolving the methods dependency on positional information. You are then delegating to another method to resolve the remaining work of updating the camera.

The code smell here would be if you keep just extending the chain of methods calling methods. The Location taking method calls the double taking method which calls the latLng taking method which finally calls something that knows how to update the camera.

Long chains are only as strong as their weakest link. Each extension of the chain increases the footprint of code that has to work or this thing breaks.

It is a far better design if each method takes the shortest possible path to solving the problem that has been presented to it. That doesn't mean each must know how to update the camera. Each should translate their position parameter type to one uniform type that can be passed to something that does know how to update the camera when presented this one type.

Do it that way and you can remove one without breaking half of everything else.

Consider:

public void moveCameraTo(Location location){
    moveCameraTo( new LatLng(location) );
}

This makes dealing with latitude and longitude LatLngs problem. The cost is it spreads knowledge of LatLng around. That might seem expensive but in my experience it's a preferable alternative to primitive obsession which is what avoiding establishing a parameter object leaves you stuck with.

If Location is refactorable but LatLng is not, consider solving this by adding a factory to Location:

moveCameraTo( location.ToLatLng() );

This also nicely avoids primitive obsession.

Answer 2

There is nothing particularly wrong with your solution.

But my personal preference would be that those methods are not that useful. And just complicate the interface of whatever object they are part off.

The void moveCameraTo(double latitude, double longitude) doesn't really simplify the code, as I see no problem simply calling moveCameraTo(new LatLng(latitude, longitude)); in it's place. This method also smells of primitive obsession.

The void moveCameraTo(Location location) could be better solved by proving Location.ToLatLng() method and calling moveCameraTo(location.ToLatLng()).

if this was C# and if such a methods were truly necessary, I would prefer them as extension methods instead of instance methods. Usage of extension methods would become really obvious if you tried abstract away and unit-test this instance. As it would be much easier to just fake out single method instead of multiple overloads with simple conversions.

I think that with this way I eliminate the responsibility of knowing what is a LatLng in another class, for example.

I see no reason why this would be a problem. As long as your code references class that contains void moveCameraTo(LatLng latLng), it still indirectly depends on LatLng. Even if that class is never directly instantiated.

And you do not need to prepare the data before calling the function.

I don't understand what you mean. If it means creating new instance or transforming classes from one into another, I see no problem with that.

Thinking about it, I feel that what I'm saying is also supported by API design of .NET itself. Historically, lots of .NET classes followed your approach of having lots of overloads with different parameters, and simple conversions inside. But that was before extension methods existed. More modern .NET classes are more light-weight in their own APIs and if there are any methods with parameter overloads, they are provided as extension methods. Older example is NLog ILogger that has dozens of overloads for writing to log. Compare that to newer Microsoft.Extensions.Logging.ILogger that has total of 3 methods (and only 1 if you count logging itself). But there are lots of helpers and various parametrizations as extension methods.

I think this answer shows that some languages would have tools to make design like this nicer. I don't know much Java, so I'm not sure if there would be any equivalent. But even using plain static methods might be an option.

Answer 3

I'm not sure what the proper name for this is, if it has one, but it is good practice. You expose multiple overloads, allowing the calling class to determine which parameter set it wants to use. As you say, another class may not know what a LatLng object is, but might know its Location.

Having one method call the other is key too, as you don't want duplicated code across those methods. As you've done, pick one method to be the one that does the work, and have the other methods call it (directly or indirectly)

Answer 4

If you mind using kind of methods, it is just method-overloading feature it is good.

But there is not eliminate the responsibility of knowing what is a LatLng. Because you are initializing LatLng latLng = new LatLng(latitude, longitude). This is totatly dependency to LatLng. (To understand why Initializing is a dependency problem, you can check Dependency Injection) Creating overloaded method just helps clients who don't care LatLng. If you mean this, it is also good but I dont think it is an approach. It is just many service methods for clients.

So, there are two options to design your architecture:

1. Create many overloaded method and provide them to client.
2. Create few overloaded methods which need parameter(s) as interface or concrete class.

I run away as possible as from creating methods which need primitive types as parameters(Option 1). Because if your business changes many time and you need to play method parameters, it is really hard to change and implement all caller function.

Instead of this, use interfaces(Dependency Injection). If you think it is cost and takes more time, then use classes and provide their mapper extension methods(Option 2).