Hello,

I mainly wanted some tips and advice on ways to optimize this code, it's actually an exercise from my class' power point file. It says:

Exercise 1

Sam and Ella's Delicatessen wants you to write a program to take orders from the Internet. Your program asks for the item, its price, and if overnight shipping is wanted. Regular shipping for items under $10 is $2.00; for items $10 or more shipping is $3.00. For overnight delivery add $5.00.

Enter the item: Tuna Salad
Enter the price: 4.50
Overnight delivery (0==no, 1==yes) 1
Invoice:
Tuna Salad 4.50
shipping 7.00
total 11.50

After speaking with my teacher and seeing an example I fixed up my program to look like this:

import java.util.Scanner;

public class Exercise1
{
   public static void main(String[] args)
   {
      double regPrice = 2.00;
      double overnightPrice = 0.00;

      Scanner keyboard = new Scanner(System.in);

      System.out.print("Enter the item: ");
      String itemName = keyboard.nextLine();

      System.out.println();

      System.out.print("Enter the price: ");
      double itemPrice = keyboard.nextDouble();

      System.out.println();

      System.out.print("Overnight delivery (0 == no, 1 == yes): ");
      int overnightCheck = keyboard.nextInt();

      System.out.println();


      if(overnightCheck >= 0 && overnightCheck <= 1)
      {
         if(overnightCheck == 1)
            overnightPrice = 5.00;
      }

      System.out.println("             Invoice:          \n"+
      "===================================\n");

      System.out.print(itemName);
      System.out.printf("       %.2f",itemPrice);

      if(itemPrice > 10)
         regPrice = 3.00;

      System.out.println();

      double totalShip = regPrice+ overnightPrice;

      System.out.printf("Shipping         %.2f",totalShip);

      System.out.println();

      double totalPrice = itemPrice + totalShip;

      System.out.printf("Total            %.2f",totalPrice);
   }
}

Recommended Answers

All 4 Replies

Line 28:

if(overnightCheck >= 0 && overnightCheck <= 1)
      {
         if(overnightCheck == 1)
            overnightPrice = 5.00;
      }

That wrapper to make sure overnightCheck is 1 or 0 is pointless. Because you are checking it again just below, and then doing nothing for the false case. What happens if a user enters 2,3, or 5 in this:

if (overnightCheck == 1) {
    overnightPrice = 5.00;
}

Nothing. So, they have to enter 1 for the price to be set. The extra guard is redundant.

Thank you for the reply and after going over the line realized how pointless it was. I figured something was off when I quickly put it together since it did seem excesive. Most likely I was over-thinking the situation when I could have used the line below without the other if statement. But, thank you very much for your reply and criticism.

import java.util.Scanner;

public class Exercise1
{
   public static void main(String[] args)
   {
      double regPrice = 2.00;
      double overnightPrice = 0.00;

      Scanner keyboard = new Scanner(System.in);

      System.out.print("Enter the item: ");
      String itemName = keyboard.nextLine();

      System.out.println();

      System.out.print("Enter the price: ");
      double itemPrice = keyboard.nextDouble();

      System.out.println();

      System.out.print("Overnight delivery (0 == no, 1 == yes): ");
      int overnightCheck = keyboard.nextInt();

      System.out.println();


      if(overnightCheck == 1)
            overnightPrice = 5.00;

      System.out.println("             Invoice:          \n"+
      "===================================\n");

      System.out.print(itemName);
      System.out.printf("       %.2f",itemPrice);

      if(itemPrice > 10)
         regPrice = 3.00;

      System.out.println();

      double totalShip = regPrice+ overnightPrice;

      System.out.printf("Shipping         %.2f",totalShip);

      System.out.println();

      double totalPrice = itemPrice + totalShip;

      System.out.printf("Total            %.2f",totalPrice);
   }
}

What you have done with the variables prices (regPrice, overnightPrice) isperfectly valid, but could be a little better.
The problem is that to find out what value is used you need to look in two widely-separated places, so it's not easy for someone else to folow that code.
It's much clearer if you don't initialise them when you declare them, then set them explicitly like

if (overnightCheck == 0)
   overnightPrice = 0.00;
else
   overnightPrice = 5.00;

Incidentally, you could also use the ? operator, as in

overnightPrice = (overnightCheck == 0) ? 0 : 5;

More importantly, think what happens if the next step in this learning is
"After processing one order, ask the user if there is another to process, and continue processing orders until the user says no." With the code as you have written it, the prices will default to their values from the previous order.

Agreed and thank you for your reply. It could definitely be expanded for that purpose and I'll read up on the ? operator a bit more. I rarely ever used that operator and it seems to go over my head since I'm used to the typical if-statements and such. But, really thank you. You're right about if another person were to look at my work it would become difficult to follow so I should take others into consideration.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.