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:
-
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,...); } }
-
Do not put concrete parameters in the constructor of
HotelRoomOperation
and let Unity resovle it. Create properties inHotelRoomOperation
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.
-
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
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