CODEWITHSUNDEEP
c#tell-dont-ask
snazzer
snazzer

Reputation: 449

Simple Scenario, How to Incorporate Tell Don't Ask?

I'm trying to model a basic scenario involving a Person and a Seat. A Person has a Status property: Sitting or Standing. A seat has a Seated property that specifies the Person that is currently sitting in it. Also, a Seat is special in that it only "accepts" certain people to sit in it. I know it sounds strange for a Seat to "accept" someone, but just imagine it prefers certain people over others.

Following "Tell, Don't Ask," How should I design the Person and Seat objects so that a Person can sit down in a Seat only when the Seat "accepts" him and also have his status changed to Sitting. My first thought was that a Person should have a SitDown method as follows:

Person.SitDown(Seat seat);

But this seems like it would require the Person class to inspect the state of the Seat before sitting in it, as well as having to update the Seat's Seated property (instead of the Seat updating the property itself):

// inside the Person class
void SitDown(Seat seat) {
    if (seat.AcceptsPlayer(this)) {
        seat.Seated = this;
        this.Status = Sitting;
    }
}

It seems better to have the Seat class handle seating a person:

Seat.SeatPerson(Person person);

// inside Seat class
void SeatPerson(Person person) {
    if (IsAccepted(person)) {
        this.Seated = person;
        person.Status = Sitting;
    }
}

But this still requires the Seat to change the person's status. Is this the way that the person's status should be updated? Should only a Person be able to change his status? How would you model this simple scenario?

Upvotes: 5

Views: 1506

Answers (6)

Dan Nick
Dan Nick

Reputation: 132

You don't need the Seat class. The Seat class keeps track of the person who is sitting. Instead you can remove the Seat class and add a new method in the Person class named isSitting() { return this.Status == Sittting; }

Upvotes: 0

GregL
GregL

Reputation: 77

Smells like you need a seating service. That accepts a seat and a person. Then decides if the operation can happen.

That way the person is only be responsible for marking itself as seated and where. The seat is only responsible for marking itself as "taken".

It's the seating service responsibility to check if the person&seat met the criteria.

Upvotes: 2

Amy B
Amy B

Reputation: 110121

Use a callback so that each class can maintain the state that it is responsible for.

public class Seat
{
  public void SeatPerson(Person person, Action successAction)
  {
    if (IsAccepted(person))
    {
      this.Seated = person;
      successAction();
    }
  }
}


public class Person
{
  public void Sit(Seat seat)
  {
    seat.SeatPerson(this, this.SitComplete);
  }

  public void SitComplete()
  {
    this.Status = Sitting;
  }
}

There's still cyclical dependency here.

Seat has a responsibility to check that the person attempting to sit is valid to do so. Seat carries a reference to the person once they have sat. Person only knows a method to try to sit in a seat.

By convention, successAction should not be held on to longer than the SeatPerson call. This guarantees that Seat cannot compromise the state of Person.

Upvotes: 1

lc.
lc.

Reputation: 116498

The problem is your model is defined with a circular dependency. There are two ways of avoiding this.

The first one doesn't exactly follow "Tell, Don't Ask" explicitly, but it gets closer to the point. We try to find out if we can sit down, then tell the chair that we're sitting in it.

void Person.SitDown(Seat seat) {
    if (seat.AcceptsPlayer(this)) {
        seat.SeatPerson(this);
        this.Status = Status.Sitting;
    }
}

void Seat.SeatPerson(Person person) {
    this.Seated = person;
}

A better way to do this (which follows the "Tell, Don't Ask" more explicitly) might be the following. We try to sit in the chair. If the chair rejects us, we know.

void Person.SitDown(Seat seat) {
    if (seat.SeatPerson(this)) {
        this.Status = Status.Sitting;
    }
    else
    {
        //Couldn't sit down!
    }
}

bool Seat.SeatPerson(Person person) {
    if (this.IsAccepted(person) && this.Seated == null) {
        this.Seated = person;
        return true;
    }
    else
    {
        return false;
    }
}

Upvotes: 1

Brann
Brann

Reputation: 32376

let the person try to seat on the seat, and update it's state depending on the success of the operation:

Just call myPerson.TrySeat(targetseat), which returns true if the sitting process succeeded.

//inside Person class
        public bool TrySeat(Seat seat)
        {
            if (seat.TrySeat(this))
            {
                Status = Sitting;
                return true;
            }
            else
            {
                return false;
            }
        }

//inside Seat class
        internal bool TrySeat(Person person)
        {
            if (CanSeat(person))
            {
                Seated = person;
                return true;
            }
            else
            {
                return false;
            }
        }

Upvotes: -1

diclophis
diclophis

Reputation: 2442

Introduce a 3rd model... Seatings that has a reference to both the seat, and the person. Then you can create an instance of that model every time someone sits down, throw in some validations for preventing two people sitting in the same seat, and maybe even throw in some timeouts (if your sitting in a seat too long, you lose it).

Upvotes: 4

Related Questions