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

Finding weight on various different planets

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

Problem

I'm hoping I could get some help in understanding as well as optimizing code! I've been working on a few exercises on some online resources, and I decided to try and take it to the next level and start using methods.

import java.util.Scanner;

public class SpaceBoxing 
{
    static Scanner keyboard = new Scanner(System.in);

    public static void main(String[] args)
    {
        double weight = getEarthWeight();       
        int choice = getPlanetChoice();
        double realWeight = computeEffectiveWeight(weight, choice);
        System.out.println("Your weight would be " + realWeight + " on that planet. ");
    }

    static double getEarthWeight()
    {
        System.out.println("Please enter your current Earth weight: ");
        return keyboard.nextDouble();
    }

    static int getPlanetChoice()
    {
        System.out.println("I have information on the following planets: ");
        System.out.println("1. Venus");
        System.out.println("2. Mars");
        System.out.println("3. Jupiter");
        System.out.println("4. Saturn");
        System.out.println("5. Uranus");
        System.out.println("6. Neptune");
        return keyboard.nextInt();
    }

    static double computeEffectiveWeight(double weight, int choice)
        {
            double realWeight = 0;
            if(choice == 1)
                realWeight = (weight * 0.78);

            else if(choice == 2)
                realWeight = (weight * 0.39);

            else if(choice == 3)
                realWeight = (weight * 2.65);

            else if(choice == 4)
                realWeight = (weight * 1.17);

            else if(choice == 5)
                realWeight = (weight * 1.05);

            else if(choice == 6)
                realWeight = (weight * 1.23);

            return realWeight;
        }
}


I've been working on calling methods and passing parameters between them, and so far I think I've got 2/3 of the methods completed except for the final one. Just a quick

Solution

it's definitely WAY above what I've learned so far

So I try...

An answer for beginners

The code is mostly alright, sure, using an enum would help and so would Java 8 features. But let's state the simple things:

public class SpaceBoxing 
{


The opening brace should be written on the same line and separated by a single space.

static Scanner keyboard = new Scanner(System.in);


You're creating a Scanner, which is Closeable and ideally should be closed. Usually, failing to do so means leaking resources and can make your program crash (e.g., you can't open a file as there are already 1000 open file descriptors).

But you're lucky: Your Scanner only wraps the underlying InputStream System.in and itself uses no system resources. So let's forget it for now.

static double computeEffectiveWeight(double weight, int choice)
    {
        double realWeight = 0;


Be consistent. Is the weight "real" or "effective"?

if(choice == 1)


Space after if (to distinguish it visually from a method call).

You'd better always use braces, even around a single statement as it's less error-prone.

realWeight = (weight * 0.78);


Needless parentheses. And also a needless variable. It'd clearer to write

if (choice == 1) {
            return weight * 0.78;

        } else if (choice == 2) {
            return weight * 0.39;

        ...

        } else {
            return 0;
        }


But you probably should throw an IllegalArgumentException instead of returning 0 for an illegal argument.

Actually, this is a clear case for a switch:

switch (choice) {
           case 1: return weight * 0.78;
           case 2: return weight * 0.39;
           ...
           default: return 0; // or better throw
       }



I'll definitely look into it more, but I got the code working, and I guess "why break it if it ain't broke"

That's a good rule in general, but after applying it consequently for one month, you'll wish you never wrote a single line. ;) Don't hesitate to change code if it can get clearer and simpler. Learn writing tests so you don't have to be afraid of change.

Simple enum usage

As the next step, I'd go for enum Planet exactly like h.j.k. did. Modify getPlanetChoice to return it like

static Planet getPlanetChoice() {
    Planet[] planets = Planet.values();
    while (true) {
        System.out.println("I have information on the following planets: ");
        System.out.println("1. Venus");
        System.out.println("2. Mars");
        System.out.println("3. Jupiter");
        System.out.println("4. Saturn");
        System.out.println("5. Uranus");
        System.out.println("6. Neptune");
        int index = keyboard.nextInt() - 1;
        if (0 <= index && index < planets.length) {
            return planets[index];
        }
        System.out.println("\nPlease try again.\n");
    }
}


and then get the needed information out of the returned Planet.

Code Snippets

public class SpaceBoxing 
{
static Scanner keyboard = new Scanner(System.in);
static double computeEffectiveWeight(double weight, int choice)
    {
        double realWeight = 0;
if(choice == 1)
realWeight = (weight * 0.78);

Context

StackExchange Code Review Q#90470, answer score: 7

Revisions (0)

No revisions yet.