HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Pay calculator for employee at Foo Corp

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fooemployeecorppayforcalculator

Problem

I completed Assignment 2 for MIT OpenCourseWare 6.092, titled "Foo Corp". The code I wrote below calculates a an employee's total pay:

```
package fooCorp;

class Employee{

private double payPerHour; //regular hourly wage
private double overTimePayPerHour; //regular hourly wage * 1.5
private double basePayHours; //hours worked under regular hourly wage
private double overTimeHours;//hours worked under overtime pay per hour
private double totalHours; //adds basePayHours and overTimeHours
private double totalPay;//base pay + overtime pay

public void setPayPerHour(double x) //takes parameter 'x' a d sets that equal to this objects payPerHour variable
{
this.payPerHour = x;
}
public double getpayPerHour() //returns this variables payPerHour variable
{
return this.payPerHour;
}

public void setOverTimePayPerHour() //calculates and sets overtime payPerHour rate
{
this.overTimePayPerHour = this.payPerHour * 1.5;
}
public double getOverTimePayPerHour() //returns overTimePayPerHour rate
{
return this.overTimePayPerHour;
}

public void setHours(int x) //sets hours worked into three of this objects variables, basePayHours, overTimeHours and totalHours
{
if(x > 40){
this.basePayHours = 40;
}
else{
this.basePayHours = x;
}
if (x > 40){
this.overTimeHours = x - 40;
}
else{
this.overTimeHours = 0;
}

this.totalHours = this.basePayHours + this.overTimeHours;

}

public double getBasePayHours()//returns this object's basePayHours value
{
return this.basePayHours;
}
public double getOverTimeHours()//returns this object's overTimeHours value
{
return this.overTimeHours;
}
public double getTotalHours() //returns this object's totalHours value
{
return this.totalHours;
}

pu

Solution

Department of Redundancy Department

There's a lot of unnecessary variables here, so cleaning that up is going to be a good way to go. Your comments indicate this pretty clearly:

private double payPerHour; //regular hourly wage
private double overTimePayPerHour; //regular hourly wage * 1.5


If overtime pay is always regular pay times 1.5, why do we need it as a separate variable? Just drop the overtime setter and have the getter do the right thing:

public double getOverTimePayPerHour() 
{
    return this.payPerHour * 1.5;
}


Similarly, basePayHours and overTimeHours can be derived from totalHours:

public double getBasePayHours()
{
    return Math.min(this.totalHours, 40);
}

public double getOvertimeHours()
{
    return Math.max(0, this.totalHours - 40);
}


totalPay can be calculated on the fly and doesn't need to be a member variable.

public double getTotalPay()
{
    return getBasePayHours() * this.payPerHour + getOvertimeHours() * this.payPerHour * 1.5;
}


Invalid states

In addition to removing redundancies, the above changes also get rid of another problem in your code - invalid state. getTotalPay() is only valid after you call calculateTotalPay(), which itself is only valid after you call setOverTimePayPerHour(). That's a lot that the user has to keep track of to use your code correctly, and is extremely error-prone.

Excessive commenting

Comments are for making otherwise difficult code clear. Have a complicated algorithm, or doing something that seems unobvious? Comment!

But when your code speaks for itself (which you should always strive for), the comments are, at best, annoying and could easily deteriorate the quality of the code over time if you update the code but forget to the update the comments.

In each of these examples, the comment is literally restating the code:

return this.totalPay; //returns this object's totalPay value
public double getTotalHours()  //returns this object's totalHours value
public double getOverTimePayPerHour() //returns overTimePayPerHour rate
// etc.


On the other hand, the following comment is incorrect:

public void setPayPerHour(double x) //takes parameter 'x' a d sets that equal to this objects payPerHour variable


It doesn't set the parameter, it sets the object's member. Although the comment itself is unnecessary. If setPayPerHour() doesn't set something related to the hourly pay, it's the function's name that's the problem, not the lack of comment.

Code Snippets

private double payPerHour; //regular hourly wage
private double overTimePayPerHour; //regular hourly wage * 1.5
public double getOverTimePayPerHour() 
{
    return this.payPerHour * 1.5;
}
public double getBasePayHours()
{
    return Math.min(this.totalHours, 40);
}

public double getOvertimeHours()
{
    return Math.max(0, this.totalHours - 40);
}
public double getTotalPay()
{
    return getBasePayHours() * this.payPerHour + getOvertimeHours() * this.payPerHour * 1.5;
}
return this.totalPay; //returns this object's totalPay value
public double getTotalHours()  //returns this object's totalHours value
public double getOverTimePayPerHour() //returns overTimePayPerHour rate
// etc.

Context

StackExchange Code Review Q#116707, answer score: 5

Revisions (0)

No revisions yet.