1
\$\begingroup\$

Im writing this simple program for school and using NumberFormat on all the variables seems very repetitive (Note that we were asked to use the NumberFormat method). Is there any way to clean this code so it doesn't look so cluttered?

import java.util.Scanner;
import java.text.NumberFormat;

public class CarPrice {

    public static void main(String[] args) {

        double priceInitial;
        double HST;
        double PDI_CHARGE = 0.02;

        String makeModel;

        Scanner scan = new Scanner(System.in);

        System.out.print("makeModel?: ");
        makeModel = scan.nextLine().toUpperCase();

        do {
            System.out.print("Price?: ");
            priceInitial = scan.nextDouble();
            if (priceInitial < 0) {
                System.out.println("Enter A Value  Above 0");
            } else {
                System.out.println(priceInitial);
                break;
            }
        } while (true);

        do {
            System.out.print("HST?: ");
            HST = scan.nextDouble();
            if (HST < 0) {
                System.out.println("Enter A Value  Above 0");
            } else {
                System.out.println(HST);
                break;
            }
        } while (true);

        double tax = priceInitial * (HST / 100) + priceInitial;

        double PDI = (tax + priceInitial) * PDI_CHARGE;
        double AfterPDI = PDI + tax;

        String PDI_Print = NumberFormat.getCurrencyInstance().format(PDI);
        String tax_Print = NumberFormat.getCurrencyInstance().format(tax);
        String priceInitial_Print = NumberFormat.getCurrencyInstance().format(priceInitial);
        String tax_Amount = NumberFormat.getPercentInstance().format(HST / 100);
        String AfterPDI_Print = NumberFormat.getCurrencyInstance().format(AfterPDI);

        System.out.println("Congrats! Your " + makeModel + " is " + priceInitial_Print +
                "\n Your total after " + tax_Amount + " HST is " + tax_Print +
                "\n with a PDI fee of " + PDI_Print +
                "\n making your total " + AfterPDI_Print);


    }


}
\$\endgroup\$
2
\$\begingroup\$

The most important approach to reduce duplicated code is to put the repetitive code into a parameterized method. In case you use an integrated Development Environment (IDE) like eclipse, IntelliJ or alike they have an automated Refactoring to assist you with that. (lookup your IDEs online help how to invoke it).

Here is how I do this step by step

The initial step is to identify the duplicated parts. In your case the two while/do loops are the first candidates. They are similar that means that there are variables and literals that differ. We need to make them looking the same so that we can replace all of them with the call to the new method to be created.

  1. To be able to do that we need to provide a unique variable scope for both parts. This is done by enclosing each of the loop in braces {}

     {
         do {
             System.out.print("Price?: ");
             priceInitial = scan.nextDouble();
             if (priceInitial < 0) {
                 System.out.println("Enter A Value  Above 0");
             } else {
                 System.out.println(priceInitial);
                 break;
             }
         } while (true);
     }
     {
         do {
             System.out.print("HST?: ");
             HST = scan.nextDouble();
             if (HST < 0) {
                 System.out.println("Enter A Value  Above 0");
             } else {
                 System.out.println(HST);
                 break;
             }
         } while (true);
     }
    
  2. Inside this braces we declare and use new variables, that have the same names in both places (it is important that the new variables are declared in each block individually and not reused in the lower block!):

     {
         String message = "Price?: ";
         double userInput = 0;
         do {
             System.out.print(message);
             userInput = scan.nextDouble();
             if (userInput < 0) {
                 System.out.println("Enter A Value  Above 0");
             } else {
                 System.out.println(userInput);
                 break;
             }
         } while (true);
         priceInitial = userInput;
     }
     {
         String message = "HST?: ";
         double userInput = 0;
         do {
             System.out.print(message);
             userInput = scan.nextDouble();
             if (userInput < 0) {
                 System.out.println("Enter A Value  Above 0");
             } else {
                 System.out.println(userInput);
                 break;
             }
         } while (true);
         HST = userInput;
     }
    
  3. Now we select all the lines in the first part that look exactly the same as in the second part. Then you can apply the extract method refactoring of the IDE. The result should look like this:

     {
         String message = "Price?: ";
         // replaces by the IDE
         double userInput = extracted(scan, message);
         priceInitial = userInput;
     }
    
    
      // new method created by the IDE behind main()
     private static double extracted(Scanner scan, String message) {
         double userInput = 0;
         do {
             System.out.print(message);
             userInput = scan.nextDouble();
             if (userInput < 0) {
                 System.out.println("Enter A Value  Above 0");
             } else {
                 System.out.println(userInput);
                 break;
             }
         } while (true);
         return userInput;
     }
    

    Of cause you should give it a better name then what the IDE suggested.

    • In case your IDE did not replace the other occurrence of the selected code itself you copy the method call from the first block the other one and delete the obsolete code:

        {
            String message = "Price?: ";
            double userInput = extracted(scan, message);
            priceInitial = userInput;
        }
        {
            String message = "HST?: ";
            double userInput = extracted(scan, message);
            HST = userInput;
        }
      
  4. Then inline the new variables in both blocks

     {
         priceInitial = extracted(scan, "Price?: ");
     }
     {
         HST = extracted(scan, "HST?: ");
     }
    
  5. And finally remove the blocks too.

     priceInitial = extracted(scan, "Price?: ");
     HST = extracted(scan, "HST?: ");
    

Why do I suggest this rather complicated approach?

  • Each single step is a very small change to your code.
  • You don't need to think about the signature of the new method beside giving it a good name.
  • After each step you can compile and run the code to check if it still does what it should. (having Unittest will be handy...)
\$\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.