Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm submitting this code in a couple of days as part of an an interview process. The company knows that I don't have any formal experience with Java. I'm hoping they are testing my ability to learn it on the fly.

I'm looking for improvements and advice as if to whether I'm following "best practice" Java.

This is my first non-trivial Java program.

The comments below explain a bit about the program. And example input/output is posted below as well.

I'm looking for any improvements. The rounding was a bit weird, as it was always up to the next .05 cents to satisfy the requirements.

The code can be tested at http://ideone.com/ZEPZVl

Thanks in advance.

/*************************************************************************
 * RUN AT: http://ideone.com/ZEPZVl
 * REVIEWED AT: 
 *
 * ASSUMPTIONS: That the input is well-formed.  A group is terminated by 
 * an empty line of form "^\\s*$".  Headings are of the form
 * "^Input (\\d):".  Monetary values are of the form 
 * "(\\sat) ([0-9]+\\.[0-9]+)".
 * 
 * MODIFYING: Strings that mark food, books and medical supplies can be set
 * in base_array.  String that mark imports can be set in import_array. 
 *
 * NOTE: This code was developed at ideone.com.  The final empty line
 * was not detected and I was required to put an END.  I assume the
 * the front end of the web-app was stripping trailing white space.
 * 
 *************************************************************************/

import java.io.InputStream;
import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

class Main {
    public static void main (String[] args) {
        try {
            Taxer taxer = new Taxer();
            taxer.run();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

class Taxer {

    private String[] import_array =  {"import"},
                     base_array = {"book", "chocolate", "pills"};
    private Scanner scanner = new Scanner(System.in);
    private final Pattern p_input = Pattern.compile("^Input (\\d):"),
                          p_money = Pattern.compile("(\\sat) ([0-9]+\\.[0-9]+)"),
                          p_empty = Pattern.compile("^\\s*$");
    private final double TAX5 = .05,
                         TAX10 = .10;
    private double tax_group_sum = 0,
                   price_group_sum = 0,
                   price,
                   tax_import,
                   tax_base;
    private String line;

    public void run() {
        while(scanner.hasNext()) {
            analyzeLine();
        }
    }

    private void analyzeLine () {
        Matcher  m_input,
                 m_money,
                 m_empty;
        line = scanner.nextLine();
        m_input = p_input.matcher(line);
        m_money = p_money.matcher(line);
        m_empty = p_empty.matcher(line);
        price = 0;
        // heading
        if (m_input.find()) {
            System.out.println("Output " + m_input.group(1) + ":" );
        // line containing money amount
        } else if (m_money.find()) {
            line = m_money.replaceFirst(": ");
            price = Double.parseDouble(m_money.group(2));
            checkTaxed();
            checkImports();
            addValues();
            printLine();
        // empty line
        } else if(m_empty.find()) {
            printTotal();
            tax_group_sum = 0;
            price_group_sum = 0;
        // catch all echos input
        } else {
            System.out.println(line);
        }
    }

    private boolean checkImports () {
        Pattern p;
        Matcher m;
        tax_import = 0;
        for (String s : import_array) {
            p = Pattern.compile(s);
            m = p.matcher(line);
            if(m.find()) {
                tax_import = roundSingleValue(price * TAX5);
                return true;
            }
        }
        return false;
    }

    private boolean checkTaxed() {
        Pattern p;
        Matcher m;
        tax_base = 0;
        for (String s : base_array) {
            p = Pattern.compile(s);
            m = p.matcher(line);
            if(m.find()) {
                return false;
            }
        }
        tax_base = roundSingleValue(price * TAX10);
        return true;
    }

    private double roundSingleValue (double value) {
        double accuracy = 20;
        value = value * accuracy;
        value = Math.ceil(value);
        value = value / accuracy;
        return value;
    }
    private void addValues () {
        double tax_total = tax_base + tax_import;
        price = price + tax_total;
        tax_group_sum += tax_total;
        price_group_sum += price;
    }

    private void printTotal () {
        System.out.printf("Sales Taxes: %.2f%n", tax_group_sum);
        System.out.printf("Total: %.2f%n%n", price_group_sum);
    }

    private void printLine () {
        System.out.print(line);
        System.out.printf("%.2f%n", price);
    }
}

/*
Input 1:
1 book at 12.49
1 music CD at 14.99
1 chocolate bar at 0.85

Input 2:
1 imported box of chocolates at 10.00
1 imported bottle of perfume at 47.50

Input 3:
1 imported bottle of perfume at 27.99
1 bottle of perfume at 18.99
1 packet of headache pills at 9.75
1 box of imported chocolates at 11.25

END
*/

/*
Output 1:
1 book: 12.49
1 music CD: 16.49
1 chocolate bar: 0.85
Sales Taxes: 1.50
Total: 29.83

Output 2:
1 imported box of chocolates: 10.50
1 imported bottle of perfume: 54.65
Sales Taxes: 7.65
Total: 65.15

Output 3:
1 imported bottle of perfume: 32.19
1 bottle of perfume: 20.89
1 packet of headache pills: 9.75
1 box of imported chocolates: 11.85
Sales Taxes: 6.70
Total: 74.68

END
*/
share|improve this question
    
Also, consider a more functional approach - instead of methods that work on instance variables (reading or mutating), use parameters and returns. This should help to limit side-effects, which can be a huge cause of program errors. –  Clockwork-Muse Nov 21 '12 at 20:46
    
they specifically asked for an oo approach –  pure_code Dec 12 '12 at 19:52
add comment

1 Answer 1

up vote 2 down vote accepted

Just a few quick notes:

  1. The catch block in the main method seems unnecessary. Throwing out exceptions has the same effect:

    public static void main(final String[] args) {
        final Taxer taxer = new Taxer();
        taxer.run();
    }
    
  2. Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

  3. Don't use floating point variables where you may need exact results:

  4. Variable name prefixes are unnecessary and uncommon in the Java world. See Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.