Using Multiple ‘Sub-types’ vs a larger single type?

At first sight, I thought that this was a terrible design, because an Instructor is a User and so is a Student, and such is-a relation would plead for inheritance.

But thinking twice, I realized that Instructor and Student are not Users, but Roles that a user could have. This would plead for composition as in your current design. And one User can eventually have several roles at once.

This composition model raises however two questions:

• are there properties which appear in several roles ? In this case, you should clarify if it is related to the User as person (e.g. address, …) or to his role (e.g. working time). In the first case these should be factored into the User class.
• Could you have a benefit of using an abstract class Role, and make Student and Instructor inherit from it ? This could for example allow to use a role container instead of hardwiring the user to predetermined fixed roles. This could cumulate several time the same role (e.g. an Instructor with several classes, each with different students) or if there are many more roles than the two you have shown us.

It seems that you’re attempting to build an application model around the relationships between your entities; however I’d suggest that a better approach would be to separate your data model and application model.

The application model should be driven by the behaviour of your application; whereas the data model should be driven by the relationships between your entities.

If you avoid letting entity relationships inform your application model, then your application model is free to evolve around patterns and structures which support its behaviour.

A possible solution to your Instructor/Student/User Data Entities could be to set up the User as an attribute of Instructor and Student.

public class Student
{
    User UserData { get; set; }

    public string CurrentGrade { get; set; }
    public DateTime NextExamDate { get; set; }
    public bool ExamPassed { get; set; }
}

public class Instructor
{
    User UserData { get; set; }

    public TimeSpan TypicalDayStart { get; set; }
    public TimeSpan TypicalDayEnd { get; set; }
    public ObservableCollection<Student> Students { get; set; }
}

Inheritance is also a valid solution (particularly if you’re using an ORM which supports inheritance, then it might even be a preferred solution); however the main goal of data modelling is to rationalise your data, and not to support the behaviour of a specific application. (Remember that in larger systems, a single data model needs to serve many applications).

You will probably want to include an overall DataModel class which handles the persistence of your date (e.g. if it’s a file, then Save/Load, or if it’s a Database/ORM then your connection string, etc.); the DataModel class would effectively serve up your rationalised data model to the rest of the application; the data entities themselves would have no application-specific behaviour whatsoever (Though some generic behaviour such as validation is fine).

Modelling your application is a completely different scenario; your application is concerned with behaviour such as Login and BookAppointment – neither of which immediately suggest classes with names such as Instructor or Student.

For these, you might consider entities such as UserAuthenticator and Calendar. These entities may have references to your rationalised data model, but the data model (with whatever complex entity relationships it needs to represent so that the application can query and update it) would stand alone.

public class Calendar
{
    private DataModel _dataModel;

    public Calendar(DataModel dataModel)
    {
        _dataModel = dataModel;
    }

    public void BookAppointment(DateTime date, TimeSpan duration)
    {
    }
}

public class Authenticator
{
    private DataModel _dataModel;

    public Authenticator(DataModel dataModel)
    {
        _dataModel = dataModel;
    }

    public void Login(string username, string password)
    {
    }
}

You might tie it all together in the main/root of your app, for example:

public static void Main(string [] args)
{
    var dataModel = new DataModel();
    dataModel.Connect("MyDatabase");

    var calendar = new Calendar(dataModel);
    var authenticator = new Authenticator(dataModel);

    var mainApp = new MainApp(calendar, authenticator);
    mainApp.Run();
}

It’s hard to answer this without knowing more details about what your program does or will do in the future, but I would agree this is probably not the way to go.

The point of having a single User class is so that you can access data and perform actions that make sense for all kinds of users, even users that happen to be Students, Instructors, Janitors and whatnot all at the same time (in some applications these may be genuinely mutually exclusive roles, but as you’ve pointed out it’s not clear that they are in your case). For instance, bookAppointment([user1, user2, user3], time, place) probably makes sense for all kinds of Users, but calculateGPA(user) probably does not make any sense on non-Student Users. This implies that your User class should contain scheduling and location information in a format that works for all User types, but not any of the grading or exam information that simply doesn’t make sense for a non-Student User. If you end up in a situation where you need the GPA but are only given a User, there should be an explicit “can I get a Student class corresponding to this User” step rather than a calculateGPA(User) method which just throws or returns null if the User is the wrong type, since that means you can’t forget about handling the non-Student case properly.

The big question that I probably cannot answer is whether Student and Instructor should be subclasses of User, or largely separate types that are retrieved using some IDs in the User class when needed for specific operations.

The benefit of making them subclasses is that if and when you want different behavior for different types of Users, you just use polymorphism. For instance, let’s pretend you want bookAppointment([user1, user2], nextSaturday) to fail if the users are Students but not if the users are Instructors. I would expect this to involve a method like User.getDefaultAvailability(day) which for Students returns false on weekends and for Instructors returns true.

The downside of making them subclasses is that you can never have a User of multiple types without running into all the usual problems with multiple inheritance. This is fine if you can be sure you’ll never need a User to be both types at once (e.g., even if someone ends up having both roles in the real world, they’d just be given two separate user accounts). If not, it may be better to give User some nullable StudentID/InstructorID/etc fields, then have User.getDefaultAvailability() explicitly check which of these IDs are null to decide whether to call isWeekend(day) or just return true. This way you can explicitly decide which ruleset to apply when a User does fit multiple types, rather than being constrained by how your language handles MI (if it handles it at all).

P.S. I am assuming that adding a new type of user will be very rare, and if it does happen it’ll come with some very non-trivial requirements, such that arguments about whether you can write a new type-of-user class without modifying the User class aren’t terribly important.