Confusion between clone method, copy constructor and getter method when inheriting

Question

I have made a class Location which looks like this:

package TruckingCompany;

public class Location 
{
    private double x;
    private double y;
    private static final double xMax=1000.0;
    private static final double xMin=-1000.0;
    private static final double yMax=1000.0;
    private static final double yMin=-1000.0;
    public Location()
    {
        setX(0.0);
        setY(0.0);
    }
    public Location(double x,double y)
    {
        setX(x);
        setY(y);
    }
    public Location(Location location)
    {
        setX(location.getX());
        setY(location.getY());
    }
    public void setX(double x)
    {
        if(x>=xMin && x<=xMax)
        this.x=x;
    }
    public void setY(double y)
    {
        if(y>=yMin && y<=yMax)
        this.y=y;
    }
    public void set(double x,double y)
    {
        setX(x);
        setY(y);
    }
    public double getX()
    {
        return x;
    }
    public double getY()
    {
        return y;
    }
    public double getDistanceFrom(Location from)
    {
        double dx,dy;
        dx=getX()-from.getX();
        dy=getY()-from.getY();
        return Math.sqrt(Math.pow(dx, 2.0)+Math.pow(dy, 2.0));
    }
    @Override
    public String toString()
    {
        return "(" + x + " , " + y + ")";
    }
}

So based on this class, I want to make a class vehicle, which inherits from Location. But it also has to implement an interface (I called it Movable), because from Vehicle I want to to make other classes, and then the company (my project simulates a trucking company) could use an ArrayList of Vehicles. I'll need to call the methods with the tag <T implements Movable>, so that's the reason why I made the interface.

So this is the class Vehicle:

package TruckingCompany;

public class Vehicle extends Location,implements Movable
{
public void setLocation(Location location)
{
    set(location.getX(),location.getY());
}
public Location getLocation()
{
    return new Location(getX(),getY());
}
}

And this is the interface Movable:

package TruckingCompany;

import java.sql.Time;

public interface Movable 
{
public void setLocation(Location location);
public Location getLocation();
public void move(Time howLong);
public void setDirection(double degrees);
public Time getReachingTime(Location to);
}

So is correct to make getLocation a getter method? Shouldn't it be a clone? I could also implement Cloneable, and make it be a clone method. Another option could be to make it a copy constructor instead of a getter method. Pragmatically talking, what options are correct?

Answer 1

Starting with everyone else's observation, do not extend Location.

Are you going to make anything else other than Vehicle to also be Movable? If not and you are simply going to make several concrete subclasses of Vehicle, then I would simply rename Movable to Vehicle. If you want to make an abstract class AbstractVehicle to have basic Vehicle implementation you can do so. Then, just make all of your vehicle types to be concrete implementations of Vehicle (or extend AbstractVehicle).

Also, by your question title and experience in C++, you might be thinking in terms of C++ idioms not as commonly found in Java. Copy constructors -- though possible and sometimes created in Java -- are not an ordinary idiom in Java. In particular, I find it odd that you would create one for Location. How often are you actually going to want to solve a problem by creating an exact duplicate of an existing Location? Maybe you have a use case in mind, but it doesn't pop out at me as being very obvious why that would be helpful or necessary. A location should be immutable, and Vehicles should get a new Location when a location changes.

Answer 2

While you can have the set and get Location methods within the Movable class, these two functions negate the advantage of extending Location within the Vehicle class. My recommendation is to not extend Location at all, but keep it as a variable.

public class Vehicle implements Movable {
    ...
    private Location myLocation;
    ...
}

This should offer the same functionality as before but enforce the idea that a Vehicle has a location, but not is a location, and the location may be modified at will without modification of the vehicle itself.

Answer 3

If you need safe publishing, make Location immutable.

Also, by making Vehicle extend Location, you're saying that a vehicle is a location, which doesn't sound right. Rather, a vehicle has a location, so I recommend you make vehicle have a private Location field and not extend Location.