8
\$\begingroup\$

This is a velocity, distance, and time calculator that I wrote not too long ago. How can I improve upon this code?

import java.util.Scanner; import javax.swing.JOptionPane; //imports
    //--------------------------BEGIN PROGRAM--------------------------------------------------------//
public class justForFun 
{public static void main(String args[]){
        int number;
        double velocity;
        double Distance;
        double Time;
    //--------------------------Start Decision Making-----------------------------------------------//
        Scanner ekeys = new Scanner(System.in);
        System.out.print("What are you trying to calculate?\npress:\n3 "
            + "for Velocity(m/s):\n2 for Distance(m):\n1 for Time(sec):\n");
        number = ekeys.nextInt();
        //--------------------------Begin SWITCH--------------------------------------------------------//
        switch(number){
    //--------------------------Solving For TIME----------------------------------------------------//
        case 1: 
                                                                            //makes a new scanner for(users input) called 'ekey' to hold Users VELOCITY input.
            Scanner ekey = new Scanner(System.in);
            System.out.print("\n\nPlease enter a velcoity in m/s:\n");
            velocity = ekey.nextDouble();
                                                                            //end of 'ekey' for scanner, change scanner variable to 'ekeyp' to hold Users DISTANCE Input.

            Scanner ekeyp = new Scanner(System.in);
            System.out.print("Please enter the distance in Meters:\n");
            Distance = ekeyp.nextDouble();
                                                                            //end of 'ekeyp' scanner. next solving for TIME.
            //calculates for time, and prints the time in seconds.
            Time = Distance/velocity;
            System.out.print("Time equals: " + Time + " Seconds");
            break;
            //--------------------------Solving For DISTANCE------------------------------------------------//
        case 2: //distance:
                                                                            //makes a new scanner for(users input) called 'key' to hold Users VELOCITY input
            Scanner key = new Scanner(System.in);
            System.out.print("\n\nPlease enter the velocity in m/s:\n");
            velocity = key.nextDouble();

                                                                            //end of 'key' for scanner, change scanner variable to 'keyp' to hold Users TIME Input.
            System.out.print("Please enter the time in Seconds:\n");
            Scanner keyp = new Scanner(System.in);
            Time = keyp.nextDouble();

                                                                            //end of 'keyp' scanner. next solving for DISTANCE.
            Distance = velocity*Time;
            System.out.print("Distance euqals: "+ Distance + " meters");
            break;
            //--------------------------Solving For VELOCITY-------------------------------------------------//
        case 3: //Velocity:
                                                                            //makes a new scanner for(users input) called 'ke' to hold Users TIME input
            Scanner ke = new Scanner(System.in);
            System.out.print("\n\nPlease enter the time in Seconds:\n");
            Time = ke.nextDouble();

                                                                            //end of 'ke' for scanner, change scanner variable to 'k' to hold Users DISTANCE Input.
            Scanner k = new Scanner(System.in);
            System.out.print("Please enter the distance in Meters:\n");
            Distance = k.nextDouble();

                                                                            //end of 'k' scanner. next solving for Velocity.
            velocity = Distance/Time;
            System.out.print("Velocity equals: "+ velocity+" m/s");
            break;
            //--------------------------IF-CASE 1,2, or 3 is not given---------------------------------------//
        default:
            JOptionPane.showMessageDialog(null, "\n\nPlease enter something, I, a Computer,"
                    + " can actually understand - Restart the program. -.");    
            //--------------------------END PROGRAM----------------------------------------------------------//



        }
    }
}
\$\endgroup\$
11
\$\begingroup\$
import java.util.Scanner; import javax.swing.JOptionPane; //imports
    //--------------------------BEGIN PROGRAM--------------------------------------------------------//
public class justForFun 
{public static void main(String args[]){
        …

It's weird that the entire program is text-based, except that the error message is shown using Swing.

It is standard practice to put each import on its own line. Not only would it be more readable, it wold also make it easier when viewing diffs between versions of the code.

The // imports and // ---BEGIN PROGRAM--- comments merely state the obvious, and just add clutter. I recommend dropping them.

The placement of the opening brace is weird.

You could come up with a more useful name for the class, even if you are just writing this code for fun.

import java.util.Scanner;
import javax.swing.JOptionPane;

public class VelocityCalculator {
    public static void main(String[] args) {
        …

        int number;
        double velocity;
        double Distance;
        double Time;
    //--------------------------Start Decision Making-----------------------------------------------//
        Scanner ekeys = new Scanner(System.in);

Variable names should start with lowercase, by convention.

You're not starting any decision making here… you're just gathering input.

Whenever possible, define the variable at the same time as the declaration. Not only would the code be more compact, it would also make it clearer what the variable is for.

          Scanner input = new Scanner(System.in);
          System.out.print(…);
          int menuChoice = input.nextInt();

Note that the default case only catches cases there the user enters an integer other than 1, 2, or 3. If it's not a number at all, then the program would still crash on .nextInt().

There is no need to instantiate a new Scanner every time you ask a question. Just reuse the same one.

A common task within your program is to print a question and expect a double to be entered. You should define a function to do that.

Here is a simple revision that addresses some of the issues mentioned above.

import java.util.Scanner;

public class VelocityCalculator {
    private static int askInt(Scanner scanner, String prompt) {
        System.out.print(prompt);
        return scanner.nextInt();
    }

    private static double askDouble(Scanner scanner, String prompt) {
        System.out.print(prompt);
        return scanner.nextDouble();
    }

    public static void main(String args[]) {
        System.out.println("What are you trying to calculate?");

        Scanner input = new Scanner(System.in);
        int menuChoice = askInt(input, 
            "Press:\n" +
            "3 for velocity (m/s)\n" +
            "2 for distance (m)\n" +
            "1 for time (s):\n");
        System.out.println();

        double dist, speed, time;
        switch (menuChoice) {
        case 1: // Solving for time
            velocity = askDouble(input, "Please enter the velocity in m/s: ");
            dist = askDouble(input, "Please enter the distance in m: ");
            time = dist / velocity;
            System.out.printf("Time equals %s seconds.\n", time);
            break;
        case 2: // Solving for distance
            velocity = askDouble(input, "Please enter the velocity in m/s: ");
            time = askDouble(input, "Please enter the time in s: ");
            dist = velocity * time;
            System.out.printf("Distance equals %s meters.\n", dist);
            break;
        case 3: // Solving for velocity
            time = askDouble(input, "Please enter the time in s: ");
            dist = askDouble(input, "Please enter the distance in m: ");
            velocity = dist / time;
            System.out.printf("Speed equals %s m/s.\n", velocity);
            break;
        default:
            System.out.println("Please enter something I, a computer, can understand.");
        }
    }
}
\$\endgroup\$
9
  • \$\begingroup\$ "The // imports and // ---BEGIN PROGRAM--- comments merely state the obvious, and just add clutter. I recommend dropping them." thats because this was my first full program in java and it made it much more easy for me to understand and thats all care about at this moment is to make it understandable. you did help in may ways, none of which i have learned form my textbook yet so i will research them, and learn to implement them. this was overall very helpful. but im also playing around with coding, trying to get an understanding of how you can implement comments and many other thingsthank you! \$\endgroup\$
    – xyz
    Sep 18 '16 at 18:05
  • \$\begingroup\$ sorry new to everything coding and stackexchange \$\endgroup\$
    – xyz
    Sep 18 '16 at 18:06
  • 3
    \$\begingroup\$ +1, one question: Is %n instead of \n a better choice for a newline character as it is platform independent? \$\endgroup\$ Sep 18 '16 at 18:14
  • \$\begingroup\$ @I'lladdcommentstomorrow That's a good suggestion. Note that the menu had \n hard-coded in it anyway, so I didn't bother trying to make it cross-platform. \$\endgroup\$ Sep 18 '16 at 18:19
  • 2
    \$\begingroup\$ For a one-dimensional problem, a double velocity; isn't wrong. The speed would be abs(velocity). \$\endgroup\$
    – Joren
    Sep 19 '16 at 6:37
6
\$\begingroup\$

The first problem that stands out with your code is the formatting. You can always paste your code into a Java code formatter/beautifier such as that found on TutorialsPoint to indent and format the code properly.


A lot of the comments seem like they are pointing out the obvious, as @200_success mentioned. Ideally, code comments should be used mostly to clarify portions of code that may be difficult to understand from just reading the code.

It can be OK to use comments for labeling portions of code in certain circumstances, although I would posit that, in many cases, if this type of comment is needed it points to a larger problem with code organization.


The entirety of your code is in your main method. Generally, the main method is supposed to be an entry point for your program, rather than being the program itself. For example, something like this:

class Main {
    public static void main(String[] args) {
        PhysicsCalculator calc = new PhysicsCalculator();
        calc.start();
    }
}

class PhysicsCalculator {
    // your calculator logic code here
}

This makes the code easier to understand, and easier to maintain. If you want to add more types of calculations, or otherwise modify your program, you don't need to modify the entry point at all, only the class that is called from there. This may not be apparent in a small program like this, but as programs grow larger, proper code organization becomes very important.


There is a lot of application logic in your switch statement. Ideally you would likely want to separate the user selection menu logic from the calculation logic, since they have different responsibilities. For a small self-contained program like this you could make private methods for each of the calculation types and pass them arguments to return the answer. In a bigger program, more likely you would create separate classes for your presentation elements (e.g.: the selection menu) and your calculation elements.

Here is a skeleton example along with a working demo on repl.it of how you might structure your calculator to separate the different functionalities into their own methods.

class PhysicsCalculator {
    public PhysicsCalculator() { }

    public void start() {
        int userSelection = getCalculationSelection();
        double result = performCalculation(userSelection);
    }

    private int getCalculationSelection() {
        Scanner input = new Scanner(System.in);
        System.out.println("What are you trying to calculate?");
        System.out.println(
            "Press:\n" +
            "3 for speed (m/s)\n" +
            "2 for distance (m)\n" +
            "1 for time (s):\n");
        int selection = input.nextInt();
        return selection;
    }

    private double performCalculation(int calculationType) {
        double result;
        switch(calculationType) {
            case 1:
                result = solveForTime();
                break;
            case 2:
                result = solveForDistance();
                break;
            case 3:
                result = solveForSpeed();
                break;
            default:
                // TODO better handling of default case
                // for now returning 0 to avoid NullPointerException
                result = 0;
                System.out.println("Please enter something I, a computer, can understand.");
        }
        return result;
    }

    private double solveForTime() {
        Scanner input = new Scanner(System.in);
        System.out.print("Please enter the speed in m/s: ");
        double speed = input.nextDouble();
        System.out.print("Please enter the distance in m: ");
        double dist = input.nextDouble();
        double time = dist / speed;
        System.out.printf("Time equals %s seconds.\n", time);
        return time;
    }

    private double solveForDistance() {
        // TODO implement this
        return 0;
    }
    private double solveForSpeed() {
        // TODO implement this
        return 0;
    }
}
\$\endgroup\$
9
  • \$\begingroup\$ i really like how you answered this one, its seems very seamless, and fits in my criteria for K(keep)I(it)S(simple)S(stupid). i love how its set up, and its in my textbook (about 400 pages more, so ill be learning this soon) thank you for your time in answering this and teaching me something new. it also seems a code is like a fingerprint, something that ive never noticed before. its like handwriting. \$\endgroup\$
    – xyz
    Sep 19 '16 at 0:01
  • 2
    \$\begingroup\$ I'm glad you liked it @SeanJ! Keep posting questions to improve your code and before you know it, you'll be helping others improve their own code from what you learned as well :-) \$\endgroup\$
    – Phrancis
    Sep 19 '16 at 1:42
  • \$\begingroup\$ @SeanJ I had made a few small errors in the code, which I now fixed. I also added a working demo link to get you started, if you like. \$\endgroup\$
    – Phrancis
    Sep 19 '16 at 1:52
  • \$\begingroup\$ nice demo, so what ive seen is even though the code is a little longer its more organized, still its more straight forward in terms of working your way through a problem step by step. this is why im loving java more and more every day! java > python for a noob like me who likes more structure \$\endgroup\$
    – xyz
    Sep 19 '16 at 2:16
  • 1
    \$\begingroup\$ thank you all!!! ive been coding in C++ for about a 3/4 a year now (way better and easier to organize code and gives a tun of freedom, and the help i got off this site really helped me throughout my learning! \$\endgroup\$
    – xyz
    Feb 11 '18 at 13:31

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.