Search code examples
oopinheritancesoftware-designsolid-principlesdesign-principles

Using inheritance with a parent class which contains empty strings for variables that are only applicable in child classes


I am trying to model geographic locations in OOP. Location types are as follows:continent, country, state, county or more specific (e.g city, town, village, all modeled as a single type). Continents have continent code, counties have continent code and country code, states have continent code, country code and adm1code, counties and those more specific than counties have continent code, country code, adm1code and adm2code. I need all locations to implement an interface with method "isContainedIn(Location loc)". This checks whether the current location is contained in loc based on the administrative code. So in case of countries, this method would first check if loc is Continent - if not, return false. If it is continent, then it will check whether the continent code of loc is the same as the continent code of the country.

I can model the locations using a single class with a type field or use inheritance. I would rather model using inheritance with a base class called location representing locations more specific than adm2 which all administrative location types (country, code, state and county) would extend (which conceptually is not right - I'm saying that town is the parent class of continent). This way I can simply override other methods too such as the equals operator (e.g. two countries are equal if the have same country codes). However, then the base class would need to include continent code, country code, adm1code and adm2code to allow me to implement the isContainedIn(Location loc) method in every case. Country code does not make sense for continents, adm1code does not make sense for countries and so on. I can have blank strings when they don't make sense but would that violate Liskov substitution principle or any other standard design principle? If so, could you suggest other design options for this problem?

EDIT: I would want a client class to get two location instances, say l1 and l2, and be able to call l1.isContainedIn(l2) without knowing the specific type of the location. Let me elaborate on how it becomes a problem if I model the classes as follows: 1. A parent class containing only the variables making sense for all locations which are name, latitude and longitude 2. Children classes (continent, country, state, county and more specific), all implementing the isContainedIn(Location loc) interface. Continent would have continent code, country would have continent code & country code and so on.

With this structure I simply can't write out the logic for isContainedIn(Location loc) in any of the child classes. For instance, a location is contained in a continent if 1) it is not a continent and 2) it has the same continent code as the continent. But Location class has no continent code. I hope this clarifies the question and thanks a lot for looking into this!

EDIT2: Here is some example code (instead of interface I have an abstract class):

abstract class Location {
  protected String name;
  protected double lat;
  protected double lng;

  @Override
  public boolean equals(Object loc) {
    if(!(loc instanceof Location)) {
      return false;
    }
    Location l = (Location) loc;
    return this.lat==l.lat && this.lng==l.lng;
  }

  abstract boolean isContainedIn(Location loc);

} 

class Continent extends Location {
  protected String continentCode;

  @Override
  public boolean equals(Object loc) {
    if(!(loc instanceof Continent)) {
      return false;
    }
    Location l = (Continent) loc;
    return this.continentCode.equals(loc.continentCode);
  }

  boolean isContainedIn(Location loc) {
    if(loc instance of Continent) {
      return false;
    }
     //the following is the logic but won't work since location has no 
       continentCode variable
     //return loc.continentCode.equals(continentCode);
  }
}

class Country extends Location {
  protected String continentCode;
  protected String countryCode;
  @Override
  public boolean equals(Object loc) {
    if(!(loc instanceof Country)) {
      return false;
    }
    Location l = (Country) loc;
    return this.continentCode.equals(loc.continentCode) && this.countryCode.equals(loc.countryCode);
  }

  boolean isContainedIn(Location loc) {
    if(loc instance of Continent|| loc instance of Country) {
      return false;
    }
     //the following is the logic but won't work since location has no 
       countryCode or continent code variable
     //return loc.continentCode.equals(continentCode) && loc.countryCode.equals(countryCode);
  }
}

//other classes will be similar

This is what the client class might look like

class ClientClass {
  void someMethod {
   Location l1 = someClass.getLocation(String...searchParameters);
  Location l2 = someClass.getLocation(String...searchParameters);
  if(l1.isContainedIn(l2)) {
    //do something
  }
}

Objective: Model the location classes, to enable the client code to use isContainedIn method without knowing the specific type of the location. Can this be done if parent class has no knowledge of continent code, country code etc?

Alternate class design

//for locations contained in counties
class Location {
  String name;
  double lat;
  double lng;
  String continentCode;
  String countryCode;
  String adm1code;
  String adm2code;
}

class Continent extends Location {
  String countryCode ="";
  String adm1code = "";
  String adm2code = "";

  public Continent(String continentCode) {
    this.continentCode = continentCode;
  }
  //all logic will work but does that violate design principles since Continent technically has no country code, adm1code or adm2code - blank strings returned for these cases?

Thanks.


Solution

  • You've fallen into the classic OOP trap of thinking that just because two things represent the same concept, here a location on the Earth's surface, that they should share a base class.

    At the start of the question, you state you want them "all modeled as a single type". The single type you've chosen, a single point Location doesn't make sense and leads you to ask in code: location1.contains(location2). How can a single point contain another? Also, you don't use the lat/long to make the decision whether a location contains another, so it's a red herring for the question. If you want to add location for another reason, it should be a property of an item, not a base class.

    So, I know it goes against your whole premise, but I'm going to propose another solution.

    interface ContinentEntity {
        Continent getContinent();
    }
    
    // Logically anything that is of a country is also of a continent 
    interface CountryEntity extends ContinentEntity {
        Country getCountry();
    
        // We can satisfy this here in Java 8
        default Continent getContinent() {
            return getCountry().getContinent();
        }
    }
    
    public final class Continent {
        private final String continentCode;
    
        public Continent(String continentCode) {
            this.continentCode = continentCode;
        }
    
        // As long as the entity reports the same continent, we are good
        // I still don't know whether it's a City or country etc, so it
        // ticks the box of being type agnostic at this point.
        public boolean contains(ContinentEntity continentEntity) {
            return this.equals(continentEntity.getContinent());
        }
    
        @Override
        public boolean equals(Object obj) {
            if (obj == null || obj.getClass() != getClass()) return false;
    
            return ((Continent) obj).continentCode.equals(continentCode);
        }
    
        @Override
        public int hashCode() {
            return continentCode.hashCode();
        }
    }
    
    public final class Country implements ContinentEntity {
        // Could be the code, but consider this over stringly type
        private final Continent continent;
        private final String coutryCode;
    
        Country(Continent continent, String coutryCode) {
            this.continent = continent;
            this.coutryCode = coutryCode;
        }
    
        @Override
        public Continent getContinent() {
            return continent;
        }
    
        public boolean contains(CountryEntity countryEntity) {
            return this.equals(countryEntity.getCountry());
        }
    
        @Override
        public boolean equals(Object obj) {
            if (obj == null || obj.getClass() != getClass()) return false;
    
            return ((Country) obj).coutryCode.equals(coutryCode);
        }
    
        @Override
        public int hashCode() {
            return coutryCode.hashCode();
        }
    }
    
    public final class City implements CountryEntity {
        private final Country country;
        private final String name;
    
        public City(Country country, String name) {
            this.country = country;
            this.name = name;
        }
    
        @Override
        public Country getCountry() {
            return country;
        }
    }
    

    So you can define a new class:

    public final class Village implements CountryEntity {
        private final Country country;
        private final String name;
    
        public Village(Country country, String name) {
            this.country = country;
            this.name = name;
        }
    
        @Override
        public Country getCountry() {
            return country;
        }
    }
    

    and you can already test country.contains(village) or continient.contains(village) without modifying any existing classes. That is a good sign that we've designed correctly. See the OCP.

    So how does that look in usage:

    Continent europe = new Continent("EUROPE");
    Country uk = new Country(europe, "UK");
    City london = new City(uk, "London");
    
    // Sensible questions compile:
    europe.contains(uk);
    uk.contains(london);
    europe.contains(london);
    
    // Non-sense does not even compile - yay for typesafety for free:
    uk.contains(europe);