4
\$\begingroup\$

Would it be possible to get some feedback on some code I've just written? I've been working my way through Deitels' Java How to Program for the past few months and thought I'd better ask for some help before my bad coding habits get too ingrained.

The current problem I've just been working on is this:

(Airline Reservations System) A small airline has just purchased a computer for its new automated reservations system. You have been asked to develop the new system. You are to write an application to assign seats on each flight of the airline’s only plane (capacity: 10 seats).

Your application should display the following alternatives:

  • "Please type 1 for First Class"
  • "Please type 2 for Economy"

If the user types 1, your application should assign a seat in the first-class section (seats 1–5). If the user types 2, your application should assign a seat in the economy section (seats 6–10). Your application should then display a boarding pass indicating the person's seat number and whether it is in the first-class or economy section of the plane.

Use a one-dimensional array of primitive type boolean to represent the seating chart of the plane. Initialize all the elements of the array to false to indicate that all the seats are empty. As each seat is assigned, set the corresponding elements of the array to true to indicate that the seat is no longer available.

Your application should never assign a seat that has already been assigned. When the economy section is full, your application should ask the person if it is acceptable to be placed in the first-class section (and vice versa). If yes, make the appropriate seat assignment. If no, display the message "Next flight leaves in 3 hours."

Below is what I've come up with. Please feel free to tear it apart.

import java.util.Scanner;

public class Airline 
{
    boolean[] seating = new boolean[11]; /* create 10 seat numbers (array[0] will not be used). Empty seat indicated by false*/
    Scanner input = new Scanner(System.in);

    public void start()
    {       
        while ( true )
        {
            makeReservation();
        }   
    }

    public void makeReservation()
    {
        System.out.println("Please type 1 for First Class or 2 for Economy: ");
        int section = input.nextInt();
        if ( section == 1 )
        {
            firstClassSeat();
        }
        else
        {
            economySeat();
        }
    }

    public void firstClassSeat() // assign a first class seat
    {
        for ( int count = 1; count <= 5; count++ )
        {
            if ( seating[count] == false )  // if false, then a seat is available for assignment
            {
                seating[count] = true;  // assign seat
                System.out.printf("First Class. Seat# %d\n", count);
                break;
            }
            else if ( seating[5] == true ) // If seating[5] is true then first class is fully booked
            {
                if ( seating[10] == true) // If seating[10] is true then economy (and therefore whole flight) is fully booked
                {
                    System.out.println("Sorry, flight fully booked. Next flight is in 3 hours.");
                }
                else // ask passenger if they would like an economy ticket instead
                {
                    System.out.println("First Class is fully booked. Would you like Economy? 1 for Yes 2 for No");
                    int choice = input.nextInt();
                    if ( choice == 1 )
                    {
                        economySeat();
                        start();
                    }
                    else
                    {
                        System.out.println("Next flight is in 3 hours.");
                        System.exit(0);
                    }
                }
            }
        }
    }   

    public void economySeat() // assign an economy seat
    {
        for ( int count = 6; count <= 10; count++ )
        {
            if ( seating[count] == false ) // if false, then a seat is available for assignment
            {
                seating[count] = true; // assign seat
                System.out.printf("Economy. Seat# %d\n", count);
                break;
            }
            else if ( seating[10] == true ) // If seating[10] is true then economy is fully booked
            {
                if ( seating[5] == true) // If seating[5] is true then first class (and therefore whole flight) is fully booked
                {
                    System.out.println("Sorry, flight fully booked. Next flight is in 3 hours.");
                    System.exit(0);
                }
                else // ask if passenger would like a first class ticket instead
                {
                    System.out.println("Economy is fully booked. Would you like First Class? 1 for Yes 2 for No");
                    int choice = input.nextInt();
                    if ( choice == 1 )
                    {
                        firstClassSeat();
                        start();
                    }
                    else
                    {
                        System.out.println("Next flight is in 3 hours");
                        System.exit(0);
                    }
                }
            }
        }
    }
}
\$\endgroup\$
5
\$\begingroup\$

I have several suggestions:

  1. For readability I would suggest to introduce enums like

    public enum SeatType {
        FIRSTCLASS, ECONOMY }
    

    If you have to make several decisions depending on the seats type SeatType.FIRSTCLASS reads better, than perhaps seatnumber<5.

  2. You are doing twice the same thing: Iterating through a partial array.

    It would be more streamlined if you write a method for both cases, like:

    private Integer tryReservation(SeatType s) {
        Integer reservation=null;
        int start=(s==SeatType.FIRSTCLASS)?firstClassStart:economyStart;
        int stop=start+capacity;
        for(int number=start;number<stop;number+=1){
            if(seats[number]==free){
                seats[number]=!free;
                reservation=number;
                break;
            }
        }
        return reservation;
    }
    

    This makes the following possible:

    private Integer makeReservation(SeatType s) {
        SeatType alternative=getAlternative(s);
        Integer reservedSeat=tryReservation(s);
        if(reservedSeat==null && !isClassFull(alternative)){
            if(alternativeWanted){
                reservedSeat=tryReservation(alternative);
            }
        } 
        return reservedSeat;
    }
    

    So your code becomes much smaller. And the number of if-else-whatelse decreases a lot.

\$\endgroup\$
1
  • \$\begingroup\$ Hi Lilith, thanks very much for taking a look at my code, I really appreciate your feedback. I have read about Enums but haven't put them into practice for myself yet so this is a good opportunity. What you say about having a single method to make reservations in both classes makes total sense also. \$\endgroup\$ – Treehouse May 9 '13 at 18:46
4
\$\begingroup\$

Another thing I would suggest is to try using boolean values without the == operator. As you write more complex expressions, you'll find that to check the value of a boolean to either 'true' or 'false' is a redundant step. For example, your code could be changed to this:

if ( !seating[count] )  // if false, then a seat is available for assignment... Notice the use of the negation operator instead of == false
        {
            seating[count] = true;  // assign seat
            System.out.printf("First Class. Seat# %d\n", count);
            break;
        }
        else if ( seating[5] ) // If seating[5] is true then first class is fully booked
        {
            if ( seating[10] )
\$\endgroup\$
2
  • 1
    \$\begingroup\$ You should also consider the naming of boolean variables. It is good practice to start them with is, has, or can. Instead of seating, I'd name the variable isOccupied. \$\endgroup\$ – nibra May 10 '13 at 16:59
  • \$\begingroup\$ Hi packersfan, thank you for your very useful comment. I really appreciate hearing the advice of you more experienced programmers. \$\endgroup\$ – Treehouse May 14 '13 at 19:04
1
\$\begingroup\$
  1. Try not to use /* this kind of comment * / at the end of a single line (like you did on line 5).

    //use this kind of comment as often as possible
    

    Developers like to use /* this kind of comment */ to comment-out blocks of code sometimes. So using them on a single line can be irritating.

  2. On line 10, you say

    while ( true )
    

    Infinite loops are usually not a good idea. It is better to have some exit condition. If the function makeReservation() returned a bool, you could use it for your loop exit condition.

\$\endgroup\$
1
  • \$\begingroup\$ Sorry for the late reply. Hey, that's great advice thank you. I'll watch out for those while (true) loops in the future. \$\endgroup\$ – Treehouse May 14 '13 at 13:54

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.