inheritance and composition confusion?

Question

I've an abstract class Area, which is subclassed as Province, Division, District and City.

Now, I need to specify in the City class in which district this city exists. So, I will have an instance of District class inside City class (Composition), so that I could pass id of a specific district to the city and that will be stored in database city tables. But, it doesn't follow the rules of composition. As District-has-City and not the other way.

And another problem is that both classes are use inheritance and composition, which I feel is not right.

I've been trying to solve this on my own for a week by googling and other stuff. But, I'm unable to solve this issue. It's my last hope i guess. How would I solve this? any example?

Answer 1

Very interesting question, but lacks one important detail - Context. What will create cities, what will access cities? What cities, districts, etc.. will be responsible for? Will they be just data entities? I have to answer to these questions, before I can help you. So lets start designing our domain model:

Client (let it be main method) will create places through CountryBuilder interface. Client will access them through Registry interface. Places will be immutable (client is not allowed to modify places data directly). The only mutation of existing place allowed to client is adding a new place to it through CountryBuilder. All places has and (as you required) knows (has name of) it's enclosing place. State has no enclosing place, but can own Districts. District has name of State and contains Cities, City can contain no places, but has names of it's owners (ZipAddress). Of course you can achieve same effect using only one abstraction Place, but then you will need to use some checking to determine what this place is, since not all places can contain other places (e.g City), not all places are contained by others (e.g State) and some places can contain other places, as well as is contained by some place (District). To avoid checking, that would be required in order to know if that place is either a City, or District, or State I've used three different abstractions. You can create State, without creating neither City nor District, but you can't create City without specifying State and District. Please read code carefully and read my comments below:

CountryClient.java This is a client class. Only two factory methods of Country class is accessible for it.

package com.ooDesign;

import com.ooDesign.Country.Country;
import com.ooDesign.Country.Country.City;
import com.ooDesign.Country.Country.District;
import com.ooDesign.Country.Country.State;
import com.ooDesign.Country.Registry.NoSuchPlaceException;
import java.util.logging.Level;
import java.util.logging.Logger;

public class CountryClient 
{
    public static void main(String[] args)
    {
        /*Creating various combinations of places.*/      
        build("ImaginaryState" , "ImaginaryDistrict", "MadCity");
        build("ImaginaryState" , "ImaginaryDistrict", "EastCity");
        build("ImaginaryState" , "ImaginaryDistrict", "WestCity");
        build("DamnGoodState" , "DamnGoodDistrict", "DamnGoodCity");
        build("ImaginaryState" , "ProgrammersDistrict", "NoLifersCity");
        build("DamnGoodState" , "DamnGoodDistrict", "DamnBadCity");
        /*"DamnGoodCity" in "DamnBadDistrict" is not the same as "DamnGoodCity" in "DamnGoodDistrict"
           since they are located in different districts. You can easily find out how to change implementation
           to not allow to build multiple cities with same name even if they are in different districts.*/
        build("DamnGoodState" , "DamnBadDistrict", "DamnGoodCity");

        /*Printing what we just created*/
        try
        {
            traverseWorld();
        } catch (NoSuchPlaceException ex)
        {
            Logger.getLogger(CountryClient.class.getName()).log(Level.SEVERE, null, ex);
        }

        /*Getting state of speciffic city (thanks to ZipCode interface)*/
        try
        {
            print(Country.registry().state("DamnGoodState").district("DamnBadDistrict").city("DamnGoodCity").zipCode().state());
        } catch (NoSuchPlaceException ex)
        {
            Logger.getLogger(CountryClient.class.getName()).log(Level.SEVERE, null, ex);
        }

    }

    static void print(String string)
    {
        System.out.println(string);
    }

    static void traverseWorld() throws NoSuchPlaceException
    {
        for(State s : Country.registry())
        {
            print("Districts in state \"" + s.name() + "\" :");
            for(District d : s)
            {
                print("   Cities in district \"" + d.name() + "\" :");
                for(City c : d)
                {
                    print("      " + c.name());
                }
            }
            print("---------------------------------------------");
        }
    }

    static void build(String state, String district, String city)
    {
        Country.builder().build().state(state).district(district).city(city);
    }

    static void build(String state, String district)
    {
        Country.builder().build().state(state).district(district);
    }

    static void build(String state)
    {
        Country.builder().build().state(state);
    }
}

Country.java Holder of data entities interfaces (City, District, State) and static factory of Accessor(Registry) and Muttator(CountryBuilder) abstractions.

package com.ooDesign.Country;

import java.util.HashMap;
import com.ooDesign.Country.Registry.NoSuchPlaceException;

public final class Country
{
    private static HashMap<String, State> states = new HashMap<>();

    public static CountryBuilder builder()
    {
        return new CountryBuilderImpl(states);
    }

    public static Registry registry()
    {
        return new RegistryImpl(states);
    }

    public interface Place
    {
        String name();
    }

    public interface State extends Place, Iterable<District>
    {
        public District district(String districtName) throws NoSuchPlaceException;
    }

    public interface District extends Place, Iterable<City>
    {
        public City city(String cityName) throws NoSuchPlaceException;
        public String inState();
    }

    public interface City extends Place
    {
        public ZipCode zipCode();
    }

    public interface ZipCode
    {
        String state();
        String district();
        String city();
    }
}

CountryBuilder.java I like this way of composed objects building because of it's readability. Then you can instantiate objects like this Builder.build().firstObject(irstparams).secondObject(secondParams)...etc

package com.ooDesign.Country;

public interface CountryBuilder
{
    public StateBuilder build();

    public interface StateBuilder
    {
       public DistrictBuilder state(String stateName);
    }

    public interface DistrictBuilder
    {
        public CityBuilder district(String districtName);
    }

    public interface CityBuilder
    {
        public void city(String cityName);
    }

}

CountryBuilderImpl.java Implementation of CountryBuilder abstraction.

package com.ooDesign.Country;

import com.ooDesign.Country.Country.State;
import static com.ooDesign.Country.Country.*;
import com.ooDesign.Country.Registry.NoSuchPlaceException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

class CountryBuilderImpl implements CountryBuilder
{
    private Map<String, State> states;

    public CountryBuilderImpl(Map<String, State> states)
    {
        this.states = states;
    }

    @Override
    public StateBuilder build()
    {
        return new StateBuilder()
        {
            @Override
            public DistrictBuilder state(String stateName)
            {

                StateImpl currentState;
                if (states.containsKey(stateName))
                {
                    currentState = (StateImpl)states.get(stateName);
                } else
                {
                    currentState = new StateImpl(stateName);
                    states.put(stateName, currentState);
                }

                return new DistrictBuilder()
                {
                    @Override
                    public CityBuilder district(String districtName)
                    {
                        DistrictImpl currentDistrict = currentState.addDistrict(districtName);

                        return new CityBuilder()
                        {
                            @Override
                            public void city(String cityName)
                            {
                                currentDistrict.addCity(cityName);
                            }
                        };
                    }
                };
            }
        };
    }

    private static class StateImpl implements State
    {

        private final Map<String, District> districts;
        private final String stateName;

        StateImpl(String stateName)
        {
            this.districts = new HashMap<>();
            this.stateName = stateName;
        }

        DistrictImpl addDistrict(String districtName)
        {
            if (!districts.containsKey(districtName))
            {
                districts.put(districtName, new DistrictImpl(stateName, districtName));
            }
            return (DistrictImpl)districts.get(districtName);
        }

        @Override
        public District district(String districtName) throws Registry.NoSuchPlaceException
        {
            if (!districts.containsKey(districtName))
            {
                throw new Registry.NoSuchPlaceException("District \"" + districtName + "\" in state of " + stateName + " does not exists");
            } else
            {
                return districts.get(districtName);
            }
        }

        @Override
        public String name()
        {
            return stateName;
        }

        @Override
        public Iterator<Country.District> iterator()
        {
            return districts.values().iterator();
        }

    }

    private static class DistrictImpl implements District
    {

        private final Map<String, Country.City> cities;
        private final String stateName, districtName;

        DistrictImpl(String stateName, String districtName)
        {
            this.cities = new HashMap<>();
            this.stateName = stateName;
            this.districtName = districtName;
        }

        void addCity(String cityName)
        {
            if (!cities.containsKey(cityName))
            {
                cities.put(cityName, new CityImpl(new ZipImpl(stateName, districtName, cityName)));
            }
        }

        @Override
        public City city(String cityName) throws NoSuchPlaceException
        {
            if (!cities.containsKey(cityName))
            {
                throw new Registry.NoSuchPlaceException("City \"" + cityName + "\" in state of " + stateName + " in district of " + districtName + " does not exists");
            } else
            {
                return cities.get(cityName);
            }
        }

        CityImpl getCity(String cityName)
        {
            return (CityImpl)cities.get(cityName);
        }

        @Override
        public String inState()
        {
            return stateName;
        }

        @Override
        public String name()
        {
            return districtName;
        }


        @Override
        public Iterator<Country.City> iterator()
        {
            return cities.values().iterator();
        }

    }

    private static class CityImpl implements City
    {

        private final Country.ZipCode zipCode;

        public CityImpl(Country.ZipCode zipCode)
        {
            this.zipCode = zipCode;
        }

        @Override
        public Country.ZipCode zipCode()
        {
            return zipCode;
        }

        @Override
        public String name()
        {
            return zipCode.city();
        }

    }

    private static class ZipImpl implements ZipCode
    {

        private final String state, district, city;

        public ZipImpl(String state, String district, String city)
        {
            this.state = state;
            this.district = district;
            this.city = city;
        }

        @Override
        public String state()
        {
            return state;
        }

        @Override
        public String district()
        {
            return district;
        }

        @Override
        public String city()
        {
            return city;
        }

        public String toString()
        {
            return "ZIP_CODE: STATE - " + state + "; DISTRICT - " + district + "; CITY - " + city;
        }
    }
}

Registry.java Used to access places.

package com.ooDesign.Country;

import com.ooDesign.Country.Country.State;
import java.util.Set;

public interface Registry extends Iterable<State>
{
    Set<String> listStates();
    State state(String stateName) throws NoSuchPlaceException;

    public static class NoSuchPlaceException extends Exception
    {
        public NoSuchPlaceException(String message)
        {
            super(message);
        }  
    }
}

RegistryImpl.java Name tells it's purpose.

package com.ooDesign.Country;

import com.ooDesign.Country.Country.State;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;

class RegistryImpl implements Registry
{
    private final Map<String, State> states;

    public RegistryImpl(Map<String, State> states)
    {
        this.states = states;
    }

    @Override
    public Set<String> listStates()
    {
        return states.keySet();
    }

    @Override
    public State state(String stateName) throws NoSuchPlaceException
    {
        if(!states.containsKey(stateName)) 
            throw new NoSuchPlaceException("State \"" + stateName + "does not exists");
        return states.get(stateName);
    }

    @Override
    public Iterator<State> iterator()
    {
        return states.values().iterator();
    }

}

As you can see, implementation is isolated from client, since they are in separate packages and implementation classes are not public. Client can only interact with them through interfaces. Interfaces has many purposes and advantages. They are core of OO design. I'll left for you to find out how to get all cities in specific state, all districts in specific state, all cities in specific district etc.. It is very easy to do. And you can implement many convenience methods, various management classes if you want (and you must if you are writing high quality, maintainable software). All of this code is just to show you a big picture of OO design. That's actually great that you are passionate enaugh to seek for answer entire week. My suggestion would be to find a good book and read it if you start to learn a new concepts. OO design and software architecture is vast. And beautiful. But you need to master it if you want to see that beauty in all its glory. Read book Holub on patterns , it will definitely help you. P.S Feel free to ask questions about code, as well as notify me if you would find a bug. Good luck!

Answer 2

I just see abstractions here. You have an Area, which could be a Province, Division, District or City, but that's about as far as the relationship goes in terms of inheritance. Of course, that does raise the question, "What would a province, division, district, or city have enough of in common with one another besides a name that would be enough to create an inheritance hierarchy for, instead of an interface?"

Remember that inheritance (and to an extent, interfaces) is always defined as an is-a relationship. That is to say, as your system stands now, a District is-an Area, and a City is-an Area. Both of those sound fine on paper, and there's nothing really wrong with that sort of representation. In fact, you could move that to an interface and that would probably work out just fine.

What you want to keep separate from that thinking is the has-a relationships, which is what composition introduces.

The relationships are likely bi-directional, but the main premise here is:

• A City has-a District.
• A City has-a Division.
• A City has-a Province.

...and all three of the above also have the same corresponding City.

It's fine to use both techniques, but you have to be clear on:

• why you're using them, and
• when it's most appropriate to use them.

In this case, it really does feel like inheritance makes the least amount of sense, since a getName() method could easily be described by a common interface between the four, and no inheritance needs to be brought in.