views:

347

answers:

6

In my application at the moment I have (as in so many other applications) an entity called Contact, which represents any person. At its most basic level this is used to represent business contacts. However it can also be used to represent employees of the company. and there are also a couple of special types of employee (let say there is one called Manager)

I am attempting to model this as an inheritance relationship which makes sense. Employees have names and addresses just like contacts, as well as a number of employment related attributes. Managers also have a number of manager specific attributes.

The difficulty comes when an employee gets promoted to a manager. Is it ok to convert the base class Employee to the inheriting class Manager? It feels wrong. I guess I would do it with a specialised constructor on Manager.

As an aside does NHibernate support this kind of behaviour? is it as simple as getting the employee, creating the manager from the employee, then saving the manager?

+4  A: 

As long as your business model matches your domain, you're doing the right thing.

However, it sounds to me like you should have something like:

Manager Promote(Employee employee)
{
   var manager = new Manager();
   //promote your employee to a manager here
   return manager;
}

inside some workflow process of some sort.

In regards to NHibernate, it sounds like you're mixing your ORM logic with your business domain. Promoting an Employee to a Manager is a business domain construct, and as such belongs in your business model. However, how NHibernate maps your Employees and your Managers into your DB has nothing to do with your business model, except for how to map them over. This definitely doesn't have anything to do with how to promote an employee to a manager, though.

Joseph
The NHibernate question was just to find out if NHibernate would complain about saving an object as a subclass when it was retrieved as the base type.
Jack Ryan
+2  A: 

I would personally have a base class with all the basic things in it and a list of roles.
Each role has it's own properties and functionalities.
The advantages are two fold:

  • It's easy to give or take a role to/from a person
  • It will allow your people to have multiple roles without you having to make 'combination classes'

If you go with single inherritance going with inherritance will soon render you with classes like "ManagerProgrammer", "ProgrammerStockManager", "ProgrammerSupport"

borisCallens
+7  A: 

I'd go with composition over inheritance in this case. If you stick with inheritance, you'll be changing classes with every promotion or demotion and every time you hire a contact or an employee leaves and becomes a regular Contact.

It's easier just to say Contacts have Roles. You can add a Manager Role to a contact to promote it and remove the Role to fire it.

Terry Wilcox
This should be marked as the correct answer
Pierreten
A: 

G'day,

If you find you have to convert a derived class from one type to another derived type then that is a smell that the initial design has problems.

My gut feeling here is that you are representing a Manager object incorrectly.

Go back to basics and think in OO terms where your base class (Contact) contains the common elements of both Employee and Manager objects. Any derived objects are just specialisations of the base class.

In this case isn't a Manager an instance of an Employee?

Both the Manager and Employee classes should have a reportsTo data member which is also of type Employee.

The only difference I can see at the moment is that a Manager object now has a collection of Employee objects that are their own directReports. This should probably be implemented as a pointer to a container of Employee objects.

I can't think of any specialisation in behaviour that needs separating out an Employee object from a Manager object.

Hmmm, maybe make the base class Person that contains Contact details in it.

Edit: Sorry, from your comment I guess I wasn't clear enough. What I described does not lead to two separate classes both directly derived from your Contact class so that you have to change an instance of an Employee to a Manager during runtime which was your original question.

That is, I don't think you should have two derived classes, an Employee and a Manager, directly inheriting from your Contact class.

Aren't both these instances of types of people who are employed by a company? Why distinguish between a Manager and an Employee? Is an Employee no longer an Employee if they become a Manager?

Having two derived classes, a Manager and an Employee, is completely wrong IMHO. Have you tried breaking things down in terms of "isa" and "has a" relationships. Then you can see that your basic structure is wrong.

To say an Employee "isa" Contact just doesn't make sense. More likely an Employee "isa" Person and a Person "has a" set of Contact details.

Maybe derive the Manager class as a specialisation of an Employee? An Employee "isa" Person. A Manager "isa" Employee which "isa" Person.

HTH

cheers,

Rob Wells
I'm sorry this doesnt really help at all. You say I am representing a manager incorrectly and then go on to describe exactly what I was proposing.
Jack Ryan
Sorry. You are still describing what I already have. an inheritance from Contact -> Employee -> Manager. The semantic difference between a person and a contact is ireelevant. The question is given this inheritance, is it ok to convert an employee to a subclass of employee (Manager).
Jack Ryan
+1  A: 

Yes, it's valid. In regards to implementation you could use:

  • Static method on Manager: public static Manager Promote(Employee employee) { ... }
  • Specialized constructor on Manager
  • Factory or service class

I think any of those approaches would be a good solution. Personally I like the specialized constructor solution since it represents the real world well: You're creating a new Manager from an existing Employee.

Jamie Ide
Don't forget Employee Demote(Manager manager) and Employee Hire(Contact contact) and Manager HireAsManager(Contact contact).
Terry Wilcox
A: 

Contact is an attribute of Employee. Worker (your Employee) is a role, Manager is a role. Worker and Manager are both still Employee, but have Roles. Role is an IS IN relationship, Employee is an AM A relationship, and Contact is a HAS A relationship. Employee HAS A Contact (1-1 relationship) One contact per employee (1-M if they have two phones etc but I digress) Employee IS IN Role (M-M relationship) Many employees many Roles Employee Is A (M-1 relatiohship) - Many employees, all of employee type.

So you are changing roles.

Mark Schultheiss