3

I am struggling with the following issue. Let's say I have a component that performs operations on guests and rooms, and uses a number of abstract interfaces, along the lines of:

class HotelManager
{
    public HotelManager(IRoomsRepository rep, ILog log, ...);

    public void PutGuestInRoom(string guest, int room, DateTime checkIn, int nights);

    public void RemoveGuestFromRoom(string guest, int room);

    public int GetSpentNights(string guest, int room);
}

"Guest", and "room" parameters appear in every method. If the logic is complicated, these parameters, as well as any related information such as GuestInfo and RoomInfo will have to be passed around a lot. So, I want to create an operation object that keeps context of current operation in its state:

class HotelRoomOperation
{
    private string _guest;
    private int _room;
    private GuestInfo _guestInfo;

    public HotelRoomOperation(string guest, int room, IRoomsRepository rep, ILog log, ...);

    public void PutGuestInRoom(DateTime checkIn, int nights);
    public void RemoveGuestFromRoom();
    public int GetSpentNights();
}

However, it is now difficult to create this object via dependency injection. Imagine that I have code like this

class HotelManager
{
    public void FillRoomWithGuests(int room, IEnumerable[string] guests) // angle brackets, anyone?
    {
        foreach (var guest in guests)
        {
            CreateHotelOperation(guest, room).PutGuestInRoom();
        }
    }
 }

How do I implement CreateHotelOperation()? I see several options, all of them bad:

  1. Explicitly "new" the operation. This will require HotelManager to know and accept all dependencies of the operation, which will create verbose code, will violate DRY principle, and encapsulation.

    class HotelManager
    {
         public HotelManager(IRoomsRepository, ILog, ...);
    
         HotelRoomOperation CreateHotelOperation(string guest, int room)
         {
             // _roomRepository not used anywhere else!
             return new HotelRoomOperation(guest, room, _roomRepository, _log,...);
         }
    }
    
  2. Do not put concrete parameters in the constructor of HotelRoomOperation and let Unity resovle it. Create properties in HotelRoomOperation to be filled at the time of the creation:

    class HotelManager
    {
         public HotelManager(Func<HotelOperation> createHotelOperation);
    
         HotelRoomOperation CreateHotelRoomOperation(string guest, int room)
         {
             var operation = _createHotelOperation();
             operation.Room = room;
             operation.Guest = guest;
             return operation;
         }
    }
    

    This is bad, because HotelRoomOperation constructor creates an object in unusable state. I can forget to initialize a property, and this will lead to an incomplete object.

  3. Similar to #2, but we have an explicit initializer on HotelRoomOperation that passes all additional properties at once:

    class HotelManager
    {
         HotelRoomOperation CreateHotelOperation(string guest, int room)
         {
             var operation = _createHotelOperation().Init(guest, room);
             return operation;
         }
    }
    
    class HotelRoomOperation
    {
         public HotelRoomOperation(/* dependencies */ );
         public HotelRoomOperation Init(string guest, int room)
         {
             _guest = guest;
             _room = room;
             return this;
         }
     }
    

    This is probably the best of all options, but HotelRoomOperation constructor still creates an object in unusable state, which is bad.

Any other ideas?

9
  • I've read and reread your question and can't see that any of it has to do with Unity Jun 13, 2016 at 0:28
  • Unity is the wizard that is supposed to call all those constructors and supply all those create functions. The outer code just does unityContainer.Resolve &lt; HotelManager &gt; Jun 13, 2016 at 2:07
  • If you're talking about Unity Container that works like any other dependency injection framework. You still have to support dependency injection in your classes. All it does is take some construction tedium off your hands. Jun 13, 2016 at 2:49
  • All that really tells me is you want to use dependency injection. Which means we shouldn't be using service locators, static references, singletons, or globals. We should ask that what we need be passed in. Use the constructor for what should needs to be shared over multiple method calls. Pass the method what can be forgotten. Jun 13, 2016 at 3:07
  • "Pass the method what can be forgotten." - this is exactly where the problem lurks. Let's call that can-be-forgotten thing "context". If your operation is complicated, you end up with a bunch of methods passing each other context and doing context.foo and context.bar. This is how we programmed in the 70's. Algorithms + data structures = programs, if you know what I mean. So, you want to be more OOP and move the logic into that Context class. But then in that logic you need dependencies, which are in the parent. What to do? Jun 13, 2016 at 11:55

1 Answer 1

1

You're leaving hotelRoomOperation in an unusable state because you've mashed two different objects together and called them hotelRoomOperation. You aren't ready for the second one to be initialized yet so you wait. While you wait, you're unusable.

Each object should have only one responsibility. That way they only exist when they can be used to fulfill that responsibility. They should also have good names that reflect this responsibility. hotelRoomOperation sounds like it could do everything from having the room cleaned to having your kidneys removed in the bathtub.

If you want to reduce the strain from passing guest room pairs around consider the parameter object pattern. Give it a meaningful name like occupant.

I see many use cases you aren't supporting. Sometimes one guest books multiple rooms. Sometimes one guest books one room but has multiple guests with them but doesn't share their information. Sometimes they give you everyone's information so anyone can request replacement keys. Sometimes a guest reserves a particular room before checking in. Up to you want you want to support. Just be aware of what you're not.

4
  • The question was really about how I initialize an object with concrete context AND dependencies without too much repetition. HotelRoomOperation is a contrived example and indeed its usefulness or even name is questionable. It does not really matter. The point is - if it is a mesh of two objects, how do I properly initiialize that second object that has concrete context of concrete guest and concrete room, plus several dependencies? Jun 13, 2016 at 2:12
  • You've yet to show the need for any of those added dependencies. You just keep shoving them in. Why does an object that associates a concrete guest with a concrete room care what's happening with the roomrepository? Jun 13, 2016 at 2:22
  • Well, it may need to call room repository to, say, figure out how many guests the room can accommodate. Again, this is just an illustration. I am not designing a hotel managing software. Attempts to redesign this particular illustration to avoid the question are not going to be an answer, unless your redesign is generic enough to eliminate the need for dependencies in any object with concrete context. Jun 13, 2016 at 2:40
  • When you're figuring out how many guests a room can accommodate the association between guest and room shouldn't even exist yet. All you know at that point is a number of guests wanting a room on a certain date over a period of time. I'm not trying to avoid the question. I'm trying to nail down what you really need so I can tease the responsibilities apart. Each object should strive to know absolutely as little as it can while still fulfilling it's one responsibility. Jun 13, 2016 at 2:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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