The problem of using composition for the is-a relationship

Question

I have a system designed for the HR system. There are accountant and programmer staff. During the first month of joining the company, the employee was not given any role. One employee can be both an accountant and a programmer. I have a design shown with the following code.

Now I need to improve the system by implementing new functionality:

Complete all accountants. (Terminate means to set the employee status as IsActive = false). The problem is that I cannot configure all accountants directly as inactive without verification. I need to check if he has any other role.

How to remake these classes to make the completion function more natural OO?

UPDATE

I am looking for an answer that has the first EF Database solution model and database schema for @AlexDev's answer.

C # code

List<Accountant> allAccountants = Get All accountants from database public class Employee { public int EmpID { get; set; } public DateTime JoinedDate { get; set; } public int Salary { get; set; } public bool IsActive { get; set; } } public class Accountant : Employee { public Employee EmployeeData { get; set; } } public class Programmer : Employee { public Employee EmployeeData { get; set; } } 

@AlexDev

 public class Employee { ... IList<Role> Roles; bool isActive; public void TerminateRole(Role role) { Roles.Remove(role); if(Roles.Count == 0) { isActive = false; } } } public class Role { abstract string Name { get;} } public class ProgrammerRole : Role { override string Name { get { return "Programmer"; } } } 

LINK

• DDD approach to access external information
• Prefer composition over inheritance?
• Inheritance and enum properties in a domain model
• Entity Framework: get subclass objects in repository

Answer 1

To use the structure that you use, you will need several inheritances for the one who is an accountant and a programmer, in addition to the fact that new roles can be added to the system, and this does not exist in C #. You should consider a different design. One of the possibilities:

 public class Employee { ... IList<Role> Roles; bool isActive; public void TerminateRole(Role role) { Roles.Remove(role); if(Roles.Count == 0) { isActive = false; } } } public class Role { abstract string Name { get;} } public class ProgrammerRole : Role { override string Name { get { return "Programmer"; } } } 

You can then subclass Role for each type, and you can decide to close only one role or all of them.

Answer 2

I am writing a new answer because from the schema that you added to the question that I assume you will not subclass the role. Also, if you use NHibernate, be sure to use public virtual properties.

 public class Employee { ... public virtual IList<Role> Roles { get; set; } public virtual bool isActive { get; set; } public virtual void TerminateRole(Role role) { Roles.Remove(role); if(Roles.Count == 0) { isActive = false; } } } public class Role { public virtual int RoleID { get; set; } public virtual string Name { get; set; } } 

And display:

 public class EmployeeMap : ClassMap<Employee> { public EmployeeMap() { Id(x => x.EmpId); Map(x => x.JoinedDate) Map(x => x.Salary); Map(x => x.IsActive); HasManyToMany(x => x.Roles).Cascade.AllDeleteOrphan().Table("EmployeeRole") } } public class RoleMap : ClassMap<Role> { public RoleMap() { Id(x => x.RoleID); Map(x => x.RoleName); } } 

Answer 3

 public abstract class AbstractEmployee { ... public abstract bool IsActiveAccountant { get; set; } public abstract bool IsActiveProgrammer { get; set; } public bool IsActive() { get { return bitwise or of all roles; } } } public class NewEmployee : AbstractEmployee { ... public override bool IsActiveAccountant { get; set; } public override bool IsActiveProgrammer { get; set; } } public class Programmer : AbstractEmployee { ... public override bool IsActiveAccountant { get; set; } public override bool IsActiveProgrammer { get; set; } } 

Minuses:

• When you add each new system-wide role, you need to change the classes

Pros:

• you do not need to look for accountants
• programmers may have an empty IsActiveAccountant implementation, because this role is inactive for them anyway.
• NewEmployee can have many roles at the same time.

If the overhead of introducing new roles is significant, I would stick to the search

Answer 4

From my answer in Prefer composition over inheritance?

First, I'll start by checking to see if there is an "is-a" relationship. If it exists, I usually check the following:

Is it possible to create a base class. That is, the base class may not be abstract. If it may not be abstract, I usually prefer composition

For example: 1. An accountant is an Employee. But I will not use inheritance, because an Employee object can be created.

For example, 2. The book is SellingItem. You cannot create an instance of SellingItem - this is an abstract concept. Therefore, I will use inheritacne. SellingItem is an abstract base class (or interface in C #)