Multiple Persons enter room simultaneously and exits after specific interval of time

Here is the link to git repo of my application I am currently implementing just to have better understanding of OOP as well as learning Concurrency (Atleast a try :D)

What is the whole simulation intends to do?

This application is intended to provide classes or interfaces to create

-> a Room in which persons can enter, objects to be interacted are placed.

-> Person who can enter a room and interact with any random object person wants to.

-> Interactable objects which can be interacted. A single object can be interacted by multiple persons at one time and some can only be interacted by one person at a time.

The code I would like to code review is allowing a person to enter in the room. So, this is what my code personEnters currently is doing.

It checks, if room is not already full, and if a person wants to SIT or STAND, if person wants to SIT, it checks for Interactable, which has SIT interaction and get the one which is free and no one using. If it gets, person SITs otherwise, person STANDs.

Until the very end, I was not returning the Person from personEnters until I came to work with multi threading on it. So, I had to return a Person object since Person is Runnable and I can start a thread on it.

simulation

More specifically this function:

```
public synchronized Person personEnters (final Person person)
throws RoomFullException, InteractionNotPossibleException, PersonNotInRoomException, NoCurrentInteraction {
Objects.requireNonNull(person);
if (persons.size() >= capacity) {
throw new RoomFullException("Room is full.");
}
if (person.getCurrentPosition() == Person.Position.SIT && getPersonsByPosition(Person.Position.SIT).size() >=
sitCapacity) {
System.out.println("No more SIT objects. So, " + person + " is going to stand.");
person.changePosition(Person.Position.STAND);
}
Sy

Exceptions and Responsibilities

throws RoomFullException, InteractionNotPossibleException, PersonNotInRoomException, NoCurrentInteraction {

Wow that's a lot of exceptions. Let's talk about the purpose of exceptions for a moment, because that definitely wasn't the point of exceptions....

Exceptions are exceptional circumstances in your program. Things that shouldn't happen but for whatever reason do. Things like files going missing, not having an internet connection, not having an object to call a method on, ...

Exceptions are not things that are expected. Things like: The user didn't add required data, The data the user tries to access was deleted, ...

Sometimes it makes sense to handle expected circumstances through exceptions. But that's rather seldom.

Ideally you'd check whether the room is full before trying to enter it, You'd check whether the object can be interacted with before trying to do so, ...

In general your code is rather clean and obvious in what it does, but your method does a lot of things. It checks whether the room is full, checks whether there's Objects the Person can sit on, and if there are makes the person try to occupy a chair.

Also it gives the Room a reference to the Person and the Person a reference to the Room.

That's too many things for a single method. Split them up into sub-methods. Consider something like:

public synchronized void Person personEnters(final Person person) {
    // omitted throws clauses for brevity
    checkRoomCapacity();
    checkSittingPossible(person);
    enterRoom(person);
    if (person.getCurrentPosition == Position.SIT) {
       tryOccupyChair(person);
    }
    return person;
}

This makes your method significantly more abstracted. Also it allows you to synchronize with finer graining, if possible. In this case here it doesn't but for other methods it might.

Nitpicks

• You violate line-lenghts in some places. Once in the throws-clause, once in your check for free seats.

• You abuse System.out as a logger. Don't do that, it's significantly slower and more inconvenient than proper logging (think about finding all errors or warnings or other logging analysis ideas). As I understand it's a toy project, so for now it's not important, but it's something you should not get into the habit of doing.

• System.out.println("Something is wrong."); is a really unhelpful error message. First off you could miss it, secondly it doesn't tell you anything.

• I'm not sold on returning the person from the method. That makes little sense to me. I'd expect this method to be a void method, especially since you're doing all your error- and precondition-checking through exceptions.

StackExchange Code Review Q#126430, answer score: 3