HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Elevator program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
elevatorprogramstackoverflow

Problem

I've been working on this elevator program for quite some time now and finally finished it and made it work and I'm pretty proud of myself, but I'd like to see ways how to optimize my code so I can learn for the future. By optimizing, I mean making the code look better, maybe by using fewer lines of code.

import java.awt.geom.*;

public class elevator
{

    static int floor = 0, choice1, person = 0;

    public static void main(String args[])
    {

        floor = ((int) (Math.random() * 10 + 1));

        System.out.println("The elevator is now on floor " +floor);
        System.out.print("Which floor are you at now (0-10) where 0 = basement: ");
        choice1 = Keyboard.readInt();

        if(floor == choice1)
        {
            System.out.println("Enter the elevator");
        }

        else if(floor > choice1)
        {
            ElevatorDown();
        }

        else if(floor  choice1)
        {
            ElevatorDown();
        }

        else if(floor =floor; floor++)

            System.out.println(floor);

        System.out.println("The elevator has arrived");
    }

    public static void ElevatorDown()
    {
        System.out.println("The elevator is on it's way down...");
        for (person = choice1; choice1<=floor; floor--)

            System.out.println(floor);

        System.out.println("The elevator has arrived");
    }
}

Solution

import java.awt.geom.*;

public class elevator


Class names are typically capitalized

{

    static int floor = 0, choice1, person = 0;


Prefer local variables to statics like this. Person doesn't appear to be used.

public static void main(String args[])
    {

        floor = ((int) (Math.random() * 10 + 1));


You've got an extra pair of parens that you don't need.

System.out.println("The elevator is now on floor " +floor);
        System.out.print("Which floor are you at now (0-10) where 0 = basement: ");
        choice1 = Keyboard.readInt();

        if(floor == choice1)
        {
            System.out.println("Enter the elevator");
        }

        else if(floor > choice1)
        {
            ElevatorDown();
        }

        else if(floor < choice1)
        {
            ElevatorUp();
        }


Rather then keeping your choice in a static variable. Pass it as a parameter to your ElevatorUp()/ElevatorDown() Movement functions.

System.out.println("To which floor would you want to go (0-10) where 0 = basement");
        choice1 = Keyboard.readInt();

        if(floor > choice1)
        {
            ElevatorDown();
        }

        else if(floor < choice1)
        {
            ElevatorUp();
        }


This is repeated from before. Write an ElevatorMove() function that decides whether to call ElevatorUp or ElevatorDown.

}

    public static void ElevatorUp()
    {
        System.out.println("The elevator is on it's way up...");

        for (person = choice1; choice1>=floor; floor++)


Declare for loop variables in the loop not some other random place. And person? What does person have to do with it? In fact person=choice1 does nothing. I'd make this a while loop.

System.out.println(floor);

        System.out.println("The elevator has arrived");
    }

    public static void ElevatorDown()
    {
        System.out.println("The elevator is on it's way down...");
        for (person = choice1; choice1<=floor; floor--)

            System.out.println(floor);

        System.out.println("The elevator has arrived");
    }


These two functions are similiar. They can be combined.
}

My reworking of your code:

import java.awt.geom.*;

public class elevator
{

    static int floor;

    public static void main(String args[])
    {

        floor = (int) (Math.random() * 10 + 1);

        System.out.println("The elevator is now on floor " +floor);
        System.out.print("Which floor are you at now (0-10) where 0 = basement: ");
        int current_floor = Keyboard.readInt();

        if(floor == current_floor)
        {
            System.out.println("Enter the elevator");
        }
        else
        {
            MoveElevator(current_floor);
        }

        System.out.println("To which floor would you want to go (0-10) where 0 = basement");
        int target_floor = Keyboard.readInt();

        MoveElevator(target_floor);
    }

    public static void MoveElevator(int target_floor)
    {
        int direction;
        if( target_floor > floor )
        {
            System.out.println("The elevator is on it's way up...");
            direction = 1;
        }else{
            System.out.println("The elevator is on it's way down...");
            direction = -1;
        }

        while(target_floor != floor)
        {
            floor += direction;
            System.out.println(floor);
        }

        System.out.println("The elevator has arrived");
    }
}

Code Snippets

import java.awt.geom.*;

public class elevator
{

    static int floor = 0, choice1, person = 0;
public static void main(String args[])
    {

        floor = ((int) (Math.random() * 10 + 1));
System.out.println("The elevator is now on floor " +floor);
        System.out.print("Which floor are you at now (0-10) where 0 = basement: ");
        choice1 = Keyboard.readInt();

        if(floor == choice1)
        {
            System.out.println("Enter the elevator");
        }

        else if(floor > choice1)
        {
            ElevatorDown();
        }

        else if(floor < choice1)
        {
            ElevatorUp();
        }
System.out.println("To which floor would you want to go (0-10) where 0 = basement");
        choice1 = Keyboard.readInt();

        if(floor > choice1)
        {
            ElevatorDown();
        }

        else if(floor < choice1)
        {
            ElevatorUp();
        }

Context

StackExchange Code Review Q#7990, answer score: 11

Revisions (0)

No revisions yet.