Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a interface with the following implementation

public abstract class PersonInterface
{
       public abstract void Update(PersonModel person);
        public abstract void Delete(int id);


        public virtual PersonModel Find(int id)
        {
            return new List<PersonModel>().Find(x => x.Id == id);
        }

        public virtual IEnumerable<Models.Person.Person> GetAll()
        {
            return new List<Models.Person.Person>();
        }
}

Since the application will handle client and employee i take that class and implement it. This is because client have a diferrent way of updating its properties and the same goes from employee

public class Client : PersonInterface{
  public override void Update(PersonModel person)
        {
           // implementing the way of how the client is going to be update it

        }

        public override void Delete(int id)
        {
            // implementing the way of how the client is going to be delete it
        }
}

And employee implementation:

public class Employee: PersonInterface{
  public override void Update(PersonModel person)
        {
           // implementing the way of how the employee is going to be update it

        }

        public override void Delete(int id)
        {
            // implementing the way of how the employee ent is going to be delete it
        }
}

And my question is: is this the correct way that solve my problem when handling cases that some classes have somethings in common but there are 2 or 3 this that may vary?

share|improve this question
    
I'd recommend making the Find and GetAll methods in abstract base class abstract as well. They current implementations provide no useful shared functionality. –  MetaFight Feb 16 at 15:11
    
If they do actually contained shared functionality, make sure the List they're working against is instantiated in the base class' constructor. –  MetaFight Feb 16 at 15:13

1 Answer 1

up vote 6 down vote accepted

Yes. When some classes have things in common but common things different having a abstract base class is a good way to go.

Having a common base class gives you a number of advantages:

  • Reuse between the classes. Your Find and GetAll methods don't have to be rewritten for each type of person. So you avoid having to test this N times, and you avoid accidental (and subtle) differences in implementation (=> bugs!). (N = the number of classes extending from Person)

  • A consistent interface. Other code using a Person doesn't (necessarily) need to know what kind of Person it's working with. For example you could write a script to get all people and update them without having to worry about the differences between clients/employees.

But you need to make sure of a few things:

  • The implementations in your base class must work for all children, now and in the future. So if you want to add an Employer class you don't want to have to change the base class methods. If you do so you have to test your other class implementations to make sure they still work. That being said, minor modifications often aren't too much of a big deal.

  • If your classes have a small amount in common but large differences you could end up only partially being able to rely on a consistent interface. So you lose the flexibility of just operating on Person objects. If your language supports it traits might be a better way to go here. Also if you find yourself in this situation your classes might be getting too big. Try breaking them down. (Nobody likes a god object!)

But broadly speaking; Yes, go for it!

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.