Java: Optimize code

Question

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 less lines of codes.

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();
    }

}

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

    for (person = choice1; choice1>=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");
}

Basically just to make less lines of code, making it look better and proper. That kind of optimization if you're following. — Lukas
– Lukas, Commented Jan 20, 2012 at 9:38
Less lines of code doesn't mean better code. Indent your code correctly, learn the Java naming conventions and stick to them, always use curly braces for your if/for/while blocks. Add Javadoc. Only declare one variable per line. Add a space after if. Don't abuse static methods, and use local variables instead of global ones. — JB Nizet
– JB Nizet, Commented Jan 20, 2012 at 9:45

dogbane · Accepted Answer · 2012-01-20 10:31:19Z

Try to make your code object-oriented. Model the elevator as an object. What can an elevator do? It can go up and down, so you need a couple of methods to do that. What properties does an elevator have? It has a current floor, which would be an instance variable. You also need a constructor to create your elevator.

Make sure you use meaningful variable names and comment your code appropriately.

Here is some code to help you:

public class Elevator {

    // the floor that the elevator is currently on
    private int currentFloor;

    /**
     * Creates an elevator at the specified floor.
     *
     * @param initialFloor the initial floor
     */
    public Elevator(int initialFloor) {
        this.currentFloor = initialFloor;
    }

    /**
     * @return the currentFloor
     */
    public int getCurrentFloor() {
        return currentFloor;
    }

    /**
     * Moves the elevator to the specified floor.
     *
     * @param floor the floor to go to.
     */
    public void goToFloor(int floor) {
        if (floor < currentFloor) {
            goDownToFloor(floor);
        } else if (floor > currentFloor) {
            goUpToFloor(floor);
        }
        System.out.println("The elevator has arrived");
    }

    /**
     * Moves the elevator up to the specified floor.
     *
     * @param floor the floor to go up to.
     */
    private void goUpToFloor(int floor) {
        System.out.println("The elevator is on its way up...");

        //TODO: put loop to go up to the floor here
    }

    /**
     * Moves the elevator down to the specified floor
     *
     * @param floor the floor to go down to.
     */
    private void goDownToFloor(int floor) {
        System.out.println("The elevator is on its way down...");

        //TODO: put loop to go down to the floor here        
    }
}

Now you need a main method which creates the elevator, reads user input and controls it. You can either create a new class for this, or add it to the Elevator class above.

public static void main(String[] args) throws Exception {

    //create an elevator at a random floor
    Elevator elevator = new Elevator(new Random().nextInt(11));

    int elevatorFloor = elevator.getCurrentFloor();
    System.out.println("The elevator is now on floor " + elevatorFloor);

    System.out.print("Which floor are you at now? (0-10) where 0 = basement: ");
    int personFloor = Keyboard.readInt();
    if(personFloor == elevatorFloor) {
        System.out.println("Enter the elevator");
    }
    else {
        elevator.goToFloor(personFloor);
    }

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

Compare this approach to your current one. It models the problem better in terms of objects and operations that you can perform on them. There is also less code duplication.

user647772 · Accepted Answer · 2012-01-20 09:39:16Z

2

Use an IDE like NetBeans or Eclipse and let it format your code.
Fix any warnings the IDE tells you about.
Add JavaDoc.
Edit: Make your code part of a class.
Edit: Follow the Java Naming Conventions

answered Jan 20, 2012 at 9:39

user647772

3 Comments

Lukas Over a year ago

Thanks, however that's what I'm doing as of now. I'm using Eclipse and I've fixed all warnings.

user647772 Over a year ago

Doesn't look like that :) No self respecting IDF would format your code like that.

Lukas Over a year ago

Well I haven't really made Eclipse format my coding, I'm simply just using Eclipse :)

Community · Accepted Answer · 2020-06-20 09:12:55Z

1

You can do alot in your code... beside to all good points Tichodroma has mentioned you need exception handling in your code.

another thing i would eliminate is doble code sections like:

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

these sections has to be extraced so its written only once. Double code sections are ususaly an indicator for code smell.

All Output sections (System.out.println...) can be extraced in a sort of output routine, in my opinion it would be better

edited Jun 20, 2020 at 9:12

CommunityBot

11 silver badge

answered Jan 20, 2012 at 9:45

CloudyMarble

37.7k70 gold badges103 silver badges133 bronze badges

Comments

dbalakirev · Accepted Answer · 2012-01-20 09:50:23Z

1

Congrats for your program!

Maybe you wanna make your program be a bit more OOP like.

For example extract the Elevator to it's own class etc...

answered Jan 20, 2012 at 9:50

dbalakirev

1,9733 gold badges21 silver badges31 bronze badges

1 Comment

dbalakirev Over a year ago

Instead let me give you some hints so you do the job :) Oops. Just saw there is a solution already above.

tim · Accepted Answer · 2012-01-20 10:06:27Z

1

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

System.out.println(floor);

This is perfectly legal, but it is quite confusing. person has nothing to do with the for loop.

for (; floor <= choice1; floor++) {
    System.out.println(floor);
}

or

while (floor <= choice1) {
    System.out.println(floor);
    floor++;
}

Are much easier to understand. If person is actually used somewhere assign choice1 before the loop, otherwise remove person.

When you change this and add the improvements the other people mentioned, your code will look quite good.

answered Jan 20, 2012 at 10:06

tim

2,0371 gold badge19 silver badges34 bronze badges

Collectives™ on Stack Overflow

Java: Optimize code

5 Answers 5

Comments

3 Comments

Comments

1 Comment

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

3 Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related