CODEWITHSUNDEEP
java
Cameron Briggs
Cameron Briggs

Reputation: 71

Sales commision not calculating in Java

I am running into a very simple problem that is driving me insane. I am trying to calculate the commission of sales and add it to the a salary but when I run the code it always gives me a commission of $0 and a compensation of $0. Here is my code I have so far:

package compensationcalculator;

import java.text.NumberFormat;

public class CompensationCalculator {

    private double salary = 75000;
    private double sales;
    private double commission;
    private double compensation;

    public CompensationCalculator() {

        this.salary = 75000.00;

    }

    public void setSales(double sales) {

        this.sales = sales;

    }

    public double getCommission() {

        this.commission = (sales * 0.2);
        return commission;

    }

    public double getCompensation() {

        this.compensation = (salary + commission);
        return compensation;

    }

    public String toString() {

        NumberFormat fmt = NumberFormat.getCurrencyInstance();

        return ("Annual Salary: " + fmt.format(salary) + "\n"
                        + "Total Sales: " + fmt.format(sales) + "\n"
                        + "Total Commission: " + fmt.format(commission) + "\n"
                        + "Total Annual Compensation: " + fmt.format(compensation));

    }

}

Now when I enter 5000 when prompted for the sales I get this as an answer after it runs:

Annual Salary: $75,000.00
Total Sales: $5,000.00
Total Commission: $0.00
Total Annual Compensation: $0.00

I am sure this is a simple error, but I have not used Java in quite some time. I have compared it to some of my really old code from years ago and can't see what I am missing. Any guidance would be very welcome!

Upvotes: 2

Views: 1536

Answers (6)

Nicolas Modrzyk
Nicolas Modrzyk

Reputation: 14197

Here is your toString, method:

public String toString() {

    NumberFormat fmt = NumberFormat.getCurrencyInstance();

    return ("Annual Salary: " + fmt.format(salary) + "\n"
                    + "Total Sales: " + fmt.format(sales) + "\n"
                    + "Total Commission: " + fmt.format(getCommission()) + "\n"
                    + "Total Annual Compensation: " + fmt.format(getCompensation()));

}

commission and compesantion variables were not set.

Upvotes: 0

Michael Aaron Safyan
Michael Aaron Safyan

Reputation: 95639

The issue here is that your constructor fails to explicitly initialize several of the fields and thus they are being initialized to their default values of 0, and you never invoke the method that computes them. (Design note: you should either initialize these in your constructor, lazily compute them without a member variable, or lazily compute and cache the value, but unless you are caching the value, you really shouldn't be doing an assignment like this in a getter. You probably also shouldn't do a caching approach unless there is a good reason to do so. Keep it simple and just always lazily calculate the result without saving the value in a member or always initialize in the constructor for something like this).

On the subject, however, I feel that it is important to point out that the double and float types are really intended for scientific computing, not monetary usages. Double will round according to binary conventions, whereas with currency, you really want to be very stringent in rounding according to the decimal system. In Java, you can use the BigDecimal class for decimal calculations.

Upvotes: 1

MadProgrammer
MadProgrammer

Reputation: 347332

The compensation value is based on the commission, which is a calculated value.

You either need to pre-calculate all the values when you set the values or you need to use the values from these methods, for example...

Pre-calculated...

public void setSales(double sales) {

    this.sales = sales;
    commission = sales * 0.2;
    compensation = salary + commission;

}

public double getCommission() {

    return commission;

}

public double getCompensation() {

    return compensation;

}

Dependency calculated...

public double getCompensation() {

    this.compensation = (salary + getCommission());
    return compensation;

}

public String toString() {

    NumberFormat fmt = NumberFormat.getCurrencyInstance();

    return ("Annual Salary: " + fmt.format(salary) + "\n"
                    + "Total Sales: " + fmt.format(sales) + "\n"
                    + "Total Commission: " + fmt.format(getCommission()) + "\n"
                    + "Total Annual Compensation: " + fmt.format(getCompensation()));

}

Upvotes: 5

paxdiablo
paxdiablo

Reputation: 882716

You need to ensure you call getCommission and getCompensation so that the member variables are set.

The simplest way to fix it is to probably set those variables within setSales so that they're always consistent.

Or ditch them altogether since they can be calculated on demand.

Simply have getWhatever simply return a calculated variable and make sure toString calls getWhatever to "print" the value.

Upvotes: 2

Scary Wombat
Scary Wombat

Reputation: 44854

In order for a method to work, you have to actually call the method.

try

return ("Annual Salary: " + fmt.format(salary) + "\n"
                    + "Total Sales: " + fmt.format(sales) + "\n"
                    + "Total Commission: " + fmt.format(getCommission ()) + "\n"
                    + "Total Annual Compensation: " + fmt.format(getCompensation ()));

Calling the variables themselves does not cause the getters methods to be called.

Upvotes: 3

Stefan S.
Stefan S.

Reputation: 4103

The methods calculating the output do not get called. Change the toString() method like this:

    return "Annual Salary: " + fmt.format(this.salary) + "\n" + "Total Sales: " + fmt.format(this.sales) + "\n"
            + "Total Commission: " + fmt.format(getCommission()) + "\n" + "Total Annual Compensation: "
            + fmt.format(getCompensation());

Upvotes: 1

Related Questions