4

I have a reservation system that allows you to Book a reservation, Modify an existing reservation, and Cancel an existing Reservation. I was looking into the Interface Segregation Principle and I was wondering how thin I should make my interfaces and if I am violation the Single Responsibility Princple. My intial design was:

interface IReservation
{
     void Book();
     void Modify();
     void Cancel(); 
}

but then I thought, what if one Reservation system, does not need to implement one of these methods for a reservation and is just concerned with Bookings for example, so I did the following:

interface IBook
{
     void Book();
}


interface IModify
{
    void Modify();
}

interface ICancel
{
    void Cancel();
}

Now I can do something like this:

interface IReservation : IBooking
{


}

or

interface IReservation : IBooking, IModify
{


}

So the question becomes am I taking it to far with thinning it out like this. Also, it becomes harder to think of names for the interface, for example, I don't like IModify or ICancel (They just seem like methods to me that should be on the IReservation interface). How do you determine what should go into an interface and what should be segegrated out into another interface, class, etc...

Charles
  • 50,943
  • 13
  • 104
  • 142
Xaisoft
  • 45,655
  • 87
  • 279
  • 432
  • It's not straightforward to tell if you're going to far with this. If you take this example literally then - yes. IMO in cases like this(small ones) the YAGNI principle decides, but if it becomes bigger then other factors become more important. – kubal5003 Oct 21 '11 at 21:00
  • Don't want to spoil your question with my limited answer, but I could say something for both approaches. The first one, being the most intuitive, everybody would start with. The second one being somewhat more like framework level interfaces, like IEnumerable etc. Is this a one time job or an attempt to create a framework? – kroonwijk Oct 21 '11 at 21:06

3 Answers3

6

You have two things you have to consider when looking at the scope of your interfaces:

  1. Does it make sense to require every IReservation to implement these members?
  2. Does it make sense to reference member X without member Y?

The first is what you've covered, and have arrived at a conclusion of "No". The second, though is just as important. Does it make sense to think of something as "can modify" without it being able to do anything else? If not, consider making IReservation and IModifiableReservation, or some other grouping of functionality.

For example, it seems like Cancel and Modify go hand-in-hand, so might want to put them both together on the IModifiableReservation, then have your class implement that interface.

As you have it, that seems a little too granular.

Adam Robinson
  • 182,639
  • 35
  • 285
  • 343
1

I would suggest to have two interfaces

interface IBookableReservation
{
     void Book();
}

and  as suggested by Adam Robinson

interface IModifiableReservation
{
   void Modify();
   void Cancel();
}

You don't need to create IReservation interface, but inherit your class directly from both IBookableReservation and IModifiableReservation. The clients can use one or both interfaces.

There is no point to create an interface, that just duplicates public methods of a single class. If interface  has the same name as a class just with "I" prefix, it's a code smell, because it indicate that it is a 1:1 relationship between the interface and the concrete classes that implement it.

See Reused Abstractions Principle (RAP)

and from http://martinfowler.com/bliki/InterfaceImplementationPair.html

Using interfaces when you aren't going to have multiple implementations is extra effort to keep everything in sync.Furthermore it hides the cases where you actually do provide multiple implementations.

Community
  • 1
  • 1
Michael Freidgeim
  • 26,542
  • 16
  • 152
  • 170
  • `Foo` could implement the `IFoo` interface because there's *also* a `FakeFoo` class that's used as a mock. It's not necessarily a code smell at all. – Daniel Mann May 11 '13 at 00:28
  • We are testing against a logical interface (rather than a keyword interface), and it does not matter whether that logical interface is provided by a class or interface. Most of movking frameworks support mocking of virtual methods. Don't bloat your codebase with interfaces that may not actually be needed. See http://stackoverflow.com/questions/12174304/is-it-recommended-to-mock-concrete-class – Michael Freidgeim May 11 '13 at 01:51
0

If your application really need to support different kind of reservations and later some common logic should be able to handle all of them - I would suggest introduce separate interface per service type and single interface per reservation itself, the idea - reservation provides set of services, so you can just expose list of services abstracted by common interface IReservationService and get rid of multiple interface implementation for each reservation system. Just create single class per service and register services via ctor of Reservation:

var reservationWithBooking = 
     new Reservation(new List<IReservationService { new BookingService() });

var reservationWithCancellation = 
     new Reservation(new List<IReservationService { new CancellationService(); });

var mixedReservation = 
     new Reservation(new List<IReservationService 
                            {
                                new BookingService(),
                                new CancellationService()
                            });

Interfaces:

interface IReservationService
{       
}

interface IBookingService : IReservationService
{
   void Book(...);
}

interface ICancellationService : IReservationService
{
   void Cancel(...);
}

interface IReservation
{
   IEnumerable<IReservationService> Services { get; }
}

class Reservation : IReservation
{
    private IList<IReservationService> services;

    public Reservation(IEnumerable<IReservationService> services)
    {
       this.services = new List<IReservationService>(services);
    }

    public IEnumerable Services<IReservationService> 
    { 
       get 
       {
          return this.services;
       }
    }
}
sll
  • 61,540
  • 22
  • 104
  • 156