0
\$\begingroup\$

I have the following task:

Implement a simple command line Java application for booking bus seats. For simplicity suppose there is only one bus, initially all seats (lets say 50) are free. Users can book one seat at time through the application API, the application itself should try to take the first available seat from 1 to 50, if any left. Make sure your application is thread-safe (i.e. many users can book tickets at the same time, but cases such booking the same seat from more than one user must be avoided) using the conventional locking mechanisms in available in Java.

The program I have written so far looks like this:

public class Bus{

    private final boolean [] seats = new boolean[50];
    private int nextSeat = 0;

    public void bookSeat() throws Exception{
        if(nextSeat<seats.length){
        seats[nextSeat]=true;
        nextSeat++;
        System.out.print("Seat number " +nextSeat+ " booked");
        }else{
            System.out.println("The bus is full sorry");
        }
        }

}

public class Passenger extends Thread{

    Bus bus;
    String passengerName;

    public Passenger(Bus bus, String passengerName){
        this.bus=bus;
        this.passengerName=passengerName;
    }

    public void run(){
        synchronized(bus){
            try {
                bus.bookSeat();
                Thread.sleep(500);
            } catch (Exception ex) {
                Logger.getLogger(Passenger.class.getName()).log(Level.SEVERE, null, ex);
            }
            System.out.println("by " + passengerName);

        }
    }

    public String getPassengerName() {
        return passengerName;
    }

    public void setPassengerName(String passengerName) {
        this.passengerName = passengerName;
    }
}

public class Main {
    public static void main(String [] args) throws InterruptedException{
        Bus someCompany = new Bus();

        Passenger p1 = new Passenger(someCompany,"Name1");
        Passenger p2 = new Passenger(someCompany, "Name2");

        p1.start();
        p2.start();

    }
}

Does my solution satisfy the task? I want to make sure to do so and to improve my solution.

\$\endgroup\$
2
  • \$\begingroup\$ You should first determine if it's working before posting it here. However, it's already been answered, so the code must stay intact. \$\endgroup\$ – Jamal Jun 19 '16 at 17:02
  • \$\begingroup\$ @imoteb Users can book one seat at time but i see only the main-method who can book seats. Currently users are unable to book seats. Is it blessed by the prof? \$\endgroup\$ – Grim Jun 19 '16 at 18:39
2
\$\begingroup\$

Yes but...

The synchronization goes against the same bus, that means that every thread in the JVM must wait for other threads who are in the synchronization-block for this bus-instance.

You solved the problem but it realy needs a review:

  1. The prof asked you to use a API. Even if there really is an API you did not named it. The prof must have a solid and clean evidence that you know what exactly an API is.
  2. A seat is not a boolean, the state of a seat can be an boolean. Please wrap the boolean in a class named "Seat".
  3. A passenger neither inherits an Thread nor is to Book a seat the only work a passenger will have. If a Passenger like to book two seats, one for himself and one for his stuff. From documentation of thread this is not possible because: It is never legal to start a thread more than once. Rename the class Passenger to Booking, you can book only once but you can book for multiple persons.
  4. As GreenNovice reason pointedly: The synchronization is not the responsibility of the Passenger, its the responsibility of the Busdriver (lets say the Bus). As GreenNovice clearly worked out, please move the synchronization into the signature of the bookSeat or into the code of bookSeat.
  5. The synchronization works well, no need for the Thread.sleep().
  6. The variable-name of a bus is someCompany? If i read a variable is named someCompany i like to sell all my stocks before i try to call someCompany.stop(). A better name of the variable might be someCompanysCar!

Theese are problems who must occour - because scientific programming is always collective work. No need for blame you.

\$\endgroup\$
1
\$\begingroup\$

No, your program does not satisfy the task.

If all your threads would use the same synchronized block, then the task would be satisfied. As it is now each thread has its own synchronized block which it is running inside.

I would have my synchronized block inside the object/class I try to protect from threading issues. in your case, the Bus class and bookSeat method

Either by synchronizing the whole method, or have a synchronized block inside the method the threads compete about.

public void synchronized method(){}

or

private Object object = new Object();
public void method(){
 synchronized(object){

 }
}

I've heard the latter method of doing it is faster, because then the threads may do stuff that's outside the synchronized block at the same time. But then you have to make sure that what you do outside the synchronized block is thread safe.

\$\endgroup\$
1
  • \$\begingroup\$ This is not correct, the synchronized block in both threads is using the same object on which to lock and therefore they will not interleave. Please read the "Synchronized Statements" section of this page \$\endgroup\$ – D.B. Jun 20 '16 at 1:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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