How to organize an OO design with two different types of users

I've two different type of users, and I've mapped them to two Java classes UserWheel and UserSea and they have a common abstract superclass called User. The data saved for these user types is about the same, but the behavior is different.

Then I created an abstract class called UserCollection with derived classes UserWheelCollection and UserSeaCollection to search for subusers or load a subuser.

Then I've added an abstract method to UserCollection class with signature

public abstract List<User> listAllSubusers()

this is because the implementation will differ. Each User created will be a UserWheel or a UserSea, depending on which method was called, but also all the rest of the implementation is quite different.

Then I want to add a new method to UserCollection with signature public User loadById(int idUser). In this case the implementation would be the same except for the fact that the User returned would be an instance of either UserWheel or UserSea. I'm reluctant in this case to use an abstract method in the base class because of code duplication.

I could check the concrete class of UserCollection with instanceof and create an appropriate subclass, but it doesn't seem object oriented and breaks the open-close principle.

Another idea would be to add an abstract method createNewUser() to UserCollection and concrete implementations in the subclasses to return a new instance, so the base class would just call this createNewUser() method.

Do you think this second path makes sense? Or you would organize things in a different way and how?

UPDATE. The current situation is:

abstract class User
   public String getAddress()
   public void setAddress()
   ...

class UserSea extends User
class UserWheel extends User

abstract class UserCollection
   protected abstract User createNewUser();
   public abstract List<User> listAllSubUsers();
   public User loadById(int idUser) {
       User newUser = createNewUser();
       //populate it
       return newUser;
   }

class UserSeaCollection
   protected User createNewUser() {
        return new UserSea();
   }
   public List<User> listAllSubusers()

class UserWheelCollection
   protected User createNewUser() {
       return new UserWheel();
   }
   public List<User> listAllSubusers()

I tried to understand the strategy pattern, as suggested by trashgod, and here is my first attempt:

interface SubuserManagement
    List<User> listAllSubUsers();
    ...

interface UserCrud
   void create();
   User readById(int idUser);
   void update();
   void delete();

class UserSeaCollection implements SubUserManagement, UserCrud

   private SubUserManagement subuserBehavior = new SubUserManagementSeaImplementation();
       private UserCrud userCrudBehavior = new UserCrud();

   void create {
       subUserBehavior.create();
   }
   ...

class UserWheelCollection implements SubUserManagement, UserCrud
       ...

class SubUserManagementWheelImplementation implements SubUserManagement
    List<User> listAllSubUsers();

class SubUserManagementSeaImplementation implements SubUserManagement
    List<User> listAll开发者_JAVA技巧SubUsers();

class UserCrudImplementation implements UserCrud //only 1 implementation
   void create();
   User readById(int idUser);
   void update();
   void delete();

In this first attempt, I've created UserCollectionWheel and UserCollectionSea that don't share anymore a common superclass, but implement the same interfaces. The actual implementation is in external classes.

Now UserCollectionWheel and UserCollectionSea are really the same class, with the only difference of the behavior that I assign to them. Alternatively I could write just one class with setters:

UserCollection userColl = new UserCollection();
userColl.setSubUserBehavior(new SubUserManagementSeaImplementation());
userColl.setCrudBehavior(new UserCrud());

But the initialization would be cumbersome, especially if I had more behavior classes. So what am I doing wrong? How to organize this properly?

UPDATE 2: I wrote a blog post with the design that I've implemented.

Instead of inheriting behavior, consider encapsulating it using interfaces in a strategy pattern. Users would differ in having either of two concrete implementations of an interface ListSubUsersStrategy, interface CreateUserStrategy, etc.

See also the related bridge pattern.

Addendum: In the example below, every user has a concrete strategy for finding sub-users. In particular, listAllSubUsers() invokes the interface method, automatically dispatching to the right concrete implementation. The pattern doesn't relieve you of writing concrete implementations of the interface, but it does de-couple them, ensuring that changing one won't break another.

Console:

A has wheel users.
B has sea users.
C has wheel users.

Code:

import java.util.ArrayList;
import java.util.List;

/** @see http://stackoverflow.com/questions/6006323 */
public class UserMain {

    private static final List<User> users = new ArrayList<User>();

    public static void main(String[] args) {
        users.add(new User("A", new WheelStrategy()));
        users.add(new User("B", new SeaStrategy()));
        users.add(new User("C", new WheelStrategy()));
        for (User user : users) {
            user.listAllSubUsers();
        }
    }

    private static class User {

        private String name;
        private SubUsersStrategy suStrategy;

        public User(String name, SubUsersStrategy suStrategy) {
            this.name = name;
            this.suStrategy = suStrategy;
        }

        public void listAllSubUsers() {
            System.out.print(name + " manages ");
            List<User> subUsers = suStrategy.getList();
        }
    }

    private interface SubUsersStrategy {

        List<User> getList();
    }

    private static class WheelStrategy implements SubUsersStrategy {

        @Override
        public List<User> getList() {
            System.out.println("wheel users.");
            return null;
        }
    }

    private static class SeaStrategy implements SubUsersStrategy {

        @Override
        public List<User> getList() {
            System.out.println("sea users.");
            return null;
        }
    }
}

FWIW, here's my take on Trashgod's Strategy pattern approach. This is not an answer, just a supporting explanation of Trashgod's answer.

public interface UserStore<T extends User> {
    public T create(int id);
    public List<T> listAll();
}

public class SeaUserStore implements UserStore<SeaUser> {
    public SeaUser create(int id) { return new SeaUser(id); }
    public List<SeaUser> listAll() { whatever }
}

// the dry counterpart of 'sea' is either 'land' or 'road', not 'wheel' :)
public class RoadUserStore implements UserStore<RoadUser> {
    public RoadUser create(int id) { return new RoadUser(id); }
    public List<RoadUser> listAll() { whatever }
}

public class UserCollection<T extends User> {
    private UserStore<T> store;
    public UserCollection(UserStore<T> store) {
        this.store = store;
    }
    public List<T> listAll() {
        return store.listAll();
    }
    public T getById(int id) {
        T user = store.create(id);
        // populate
        return user;
    }
}

This leaves it up to the client to create the UserCollection. You could wrap that; make the UserCollection constructor private, and add:

public class UserCollection<T extends User> {
    public static UserCollection<SeaUser> createSeaUserCollection() {
        return new UserCollection<SeaUser>(new SeaUserStore());
    }
    public static UserCollection<RoadUser> createRoadUserCollection() {
        return new UserCollection<RoadUser>(new RoadUserStore());
    }
}

I could check the concrete class of UserCollection with instanceof and create an appropriate subclass, but it doesn't seem object oriented and breaks the open-close principle.

This would break Liskovs Substitution Principle, which really is a rule to check that one has a sound object hierarchy. The rule says that if a method expects a User as an argument, it should not matter if the user is a CompledUser, DirtyUseror any other user. (Hence you may not have any ifs checking instanceof etc).

Inheritance is all about "is-a" relationships, which basically means that you should be able to take any derived object and pass it to a method which expects the base class. If you can't do that, you are in a lot of trouble. Problems like this is because you have failed with your classes.

Another idea would be to add an abstract method createNewUser() to UserCollection and concrete implementations in the subclasses to return a new instance, so the base class would just call this createNewUser() method

This is a a better approach, since the caller if createNewUser doesn't care about which kind of user it get. All it knows is that it is a user.

The approach is called factory method pattern.