Potential abuse of observer pattern

Question

I have a MeetingRoom object that contains a list of Employees, there is a function on each employee called SetMeetingDate, this method has to take into account the state of all other employees in the MeetingRoom in particicular the IsReadyForMeeting property.

An individual employee for example can only successfully call SetMeetingDate(Date date) when every employee in the ChatRoom has IsReadyForMeeting set to true.

I have thought about a couple of ways to implement this functionality, I can do something where the employee checks the state of the other employees by going via the MeetingRoom object like this:

    public void SetMeetingDate(DateTime date)
    {
        if (!MeetingRoom.AllEmployeesAreReady)
        {
            throw new ApplicationException("all employees are not ready");
        }

        MeetingDate = date;
    }

which seems to me to have the responsibility in the wrong place.

A second approach that I can use is to set up an observer so that the Employee class will inform the MeetingRoom object that it has set a meeting date and the MeetingRoom class will raise the exception if MeetingRoom.AllEmployeesAreReady is false thus moving the responsibility to the MeetingRoom object and the code for the employee would look like this:

    public void SetMeetingDate(DateTime date)
    {
        MeetingDate = date;

        Observer.MeetingDateAdded();
    }

And the Meeting Room would then have a method like this:

    public void MeetingDateAdded()
    {
        if (!AllEmployeesAreReady)
        {
            throw new ApplicationException("all employees are not ready");
        }
    }

This use of the observer has been met with opposition from other developers in my team and I can see why but I can't think of a better way to implement the requirement, is there an easy/different approach that can be used in this situation?

Answer 1

This doesn't look like a reasonable place to apply the Observer Pattern.

What I do notice is that all of your examples reference local state, so I assume they are part of some sort of Meeting object. Consequently, the code

public void SetMeetingDate(DateTime date)
{
    if (!MeetingRoom.AllEmployeesAreReady)
    {
        throw new ApplicationException("all employees are not ready");
    }

    MeetingDate = date;
}

is perfectly valid, since MeetingRoom and MeetingDate are clearly local properties of your "Meeting" class.

If you prefer something a bit more self-contained, then do something like this:

public static MeetingDate SetMeetingDate(MeetingRoom room, DateTime date)
{
    if (!room.AllEmployeesAreReady)
        throw new InvalidOperationException("All employees are not ready");

    return new MeetingDate(date, room.Employees);
}

or somesuch.

Answer 2

You should go with your first solution.

Your first solution is better because it's simpler (but maybe show the user a message instead of throwing an exception if it seems more appropriate).

Head First Design Patterns says it best:

First of all, when you design, solve things in the simplest way possible. Your goal should be simplicity, not "how can I apply a pattern to this problem." Don't feel like you aren't a sophisticated developer if you don't use a pattern to solve a problem. Other developers will appreciate and admire the simplicity of your design. That said, sometimes the best way to keep your design simple and flexible is to use a pattern.

It's usually best to use design patterns only when your alternatives all seem far too complicated. Let KISS and YAGNI be your guides when it comes to deciding whether to use a design pattern.

Answer 3

I'd take a different approach, hopefully a clearer one.

// This is a filter function, needlessly wordy in Java.
List<Employee> getUnreadyParticipants(DateTime meetingMoment) {
  ArrayList<Employee> unreadyParticipants = new ArrayList<>();
  for (Employee e : this.getMeetingParticipants()) {
    if (!e.readyForMeeting(meetingTime)) unreadyParticipants.add(e);
  }
  return unreadyParticipants;
}

public void setMeetingDate(DateTime meetingMoment) {
  ArrayList<Employee> unreadyParticipants = getUnreadyParticipants(meetingMoment);
  if (unreadyParticipants.size() > 0) {
    throw new InvalidOperationException(
      "Participants not ready: " + printListNicely(unreadyParticipants)
    );
  }
  // whatever else is the logic
}

Reasons:

• Checking time compatibility is MeetingRoom's concern, not Employee's.
• The list of unready participants is available for diagnostics.
• There are no flags and other pieces of state that denote readiness (like .AreAllEmployeesReady) which may go out of sync with the list of participants.