5
\$\begingroup\$

I've only been learning Java for two weeks, and I had to complete this task as part of my course. My instructor could not give specific feedback as answer file is already provided, the code works but I am just wondering what some of you might think of my code if it can be refined. It's a basic pizza ordering program.

The requirements were:

able to specify pizzas one at a time using an loop. loop can end or not based on input. order viauser input using the Scanner class. format currency with NumberFormat Thank you for your time and wisdom.

import java.util.Scanner;  
import java.text.NumberFormat; 
        
public class PizzaOrder {
   
    public static void main(String[] args) {
        //defining and initialising variables.
        NumberFormat nf = NumberFormat.getCurrencyInstance();
        Scanner keyboard = new Scanner(System.in);
        int pizzaquantity=0;
        int numberoftoppings = 0;
        double toppingtotalprice =0;
        char ordermore = 'n';
        double runningtotal = 0;
        final double stp = 1.0, ltp = 1.5, ftp = 2.0;
        final double s = 8.0, l = 11.0, f = 14.0;
        
        //using a do while loop, because condition to stop is at the end.        
        do{
            
            System.out.println("Please place the order for your pizza.");
            System.out.println("Size (s = small, l = large, f = family):");
            char pizzasize = keyboard.next().charAt(0);
            //if else-else if nested inside loop
            //one iteration of the conditional statement for pizza size type.
            //number format is applied to every iteration of cost.
            if (pizzasize == 's')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * stp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * s);
                System.out.println("Cost is: " + nf.format(runningtotal));
                
                
            }
            else if (pizzasize == 'l')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * ltp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * l);
                System.out.println("Cost is:" + nf.format(runningtotal));
                
                
            }
            else if   (pizzasize == 'f')
            {
                System.out.println("Number of toppings?:");
                numberoftoppings = keyboard.nextInt();
                toppingtotalprice = (numberoftoppings * ftp);
                System.out.println("Pizza Quantity:");
                pizzaquantity = keyboard.nextInt();
                runningtotal = (pizzaquantity * toppingtotalprice * l);
                System.out.println("Cost is:" + nf.format(runningtotal));
                
                
            }
            else 
            {
            }        
                System.out.println("Order more? (y/n):");
                ordermore = keyboard.next().charAt(0);
        }
        while (ordermore != 'n');
         
        
        System.out.println("Total cost is: " + nf.format(runningtotal += runningtotal));
        System.out.println("Thank you for your order!");
    }
    
}
\$\endgroup\$
4
\$\begingroup\$

The first thing that I observed was the duplication. Whatever the size of a pizza you have to ask for the same things. You should figure out how to remove that duplication of code.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks @pacmaninbw for the edit, much better than my hard words. +1 \$\endgroup\$ – gervais.b Sep 21 at 14:53
  • \$\begingroup\$ Word usage, it should have been hit rather than hurt. \$\endgroup\$ – pacmaninbw Sep 21 at 14:58
4
\$\begingroup\$
NumberFormat nf = NumberFormat.getCurrencyInstance();

Don't shorten names just because you can, it makes the code harder to read and maintain.

final double s = 8.0, l = 11.0, f = 14.0;

Perfect example of that. The more descriptive the names are, the better. Length should not be of any concern.


double toppingtotalprice =0;

A double is still a floating point. For money, you always want to use BigDecimal, or an int as the smallest unit your money has (Cents for example).


char ordermore = 'n';

Should mot likely be a boolean.


        //using a do while loop, because condition to stop is at the end.        
        do{

You can use a while here, though.


I'll skip the rest of your code and directly skip to the part that you've been waiting for: That works, but is bad design.

Note that in the following examples I'm still using double for simplicity, you should use something else.

Let's start with the fact that you have a lot of repetition, extract common parts into a function which accepts parameters to parameterize it. For example:

private static final double inputPizza(double pizzaSizePrice, double toppingPricel) {
    System.out.println("Number of toppings?:");
    numberoftoppings = keyboard.nextInt();
    toppingtotalprice = (numberoftoppings * toppingPrice);
    System.out.println("Pizza Quantity:");
    pizzaquantity = keyboard.nextInt();
    return (pizzaquantity * toppingtotalprice * pizzaSizePrice);
}

That already reduces repetition a lot.

Now, we're going to encapsulate our information in a class of their own:

public class PizzaCosts {
    protected double price;
    protected double toppingPrice;
    
    public PizzaPriceValues(double price, double toppingPrice) {
        super();
        
        this.price = price;
        this.toppingPrice = toppingPrice;
    }
    
    public double getPrice() {
        return price;
    }
    
    public double getToppingPrice() {
        return toppingPrice;
    }
}

And in our main function:

PizzaCosts smallPizzaCosts = new PizzaCosts(8.0d, 1.0d);
PizzaCosts mediumPizzaCosts = new PizzaCosts(11.0d, 1.5d);
PizzaCosts largePizzaCosts = new PizzaCosts(14.0d, 2.d);

That groups the values nicely together and makes it easier to read. Now let's go one step further, we'll put them into a Map to access them dynamically based on user-input:

Map<String, PizzaCosts> pizzaCosts = new HashMap<>();
pizzaCosts.put("s", new PizzaCosts(8.0d, 1.0d));
pizzaCosts.put("m", new PizzaCosts(11.0d, 1.5d));
pizzaCosts.put("l", new PizzaCosts(14.0d, 2.0d));

PizzaCosts selectedPizzaCosts = pizzaCosts.get(keyboard.next());

if (selectedPizzaCosts != null) {
    runningTotal = runningTotal + inputPizza(pizzaCosts);
} else {
    // Invalid selection
}

Now that's a lot nicer, and should be enough to get you started further.

| improve this answer | |
\$\endgroup\$

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.