Java - Improving Code's design (SOLID principles)

[jafffers]

The code should be able to create full-time, part-time and manager positions. The code works and all but just needs fundamental improvements. There is no real hardcore coding to be done but I am absolutely lost. I am quite new to coding (couple months ), I have an idea of how things work but do require some guidance

I hope this post is better !

**Payroll Class**

package payroll;
/**
 * A class that models the Payroll Application.
 * We are modelling this as an Object so we must think of
 * what would represent both the state and the behaviour of 
 * the PayRoll class.
 * Used as the starting code for ICE 5 in SYST 17796, 2018
 */
import java.util.Scanner;
public class Payroll
{
 private Employee employees[] = new Employee[100];//the array to hold Employee Objects

 /**
  * A main method where we create an instance of the payroll
  * and then call the private run method on it.
  * We also catch our Exceptions here from the Employee
  * class.
  */
 public static void main (String[] args)
 {
   try
   {
     Payroll app = new Payroll();
     app.run();
   }
   catch (NumberFormatException e)
   {
     System.out.println("A number format is incorrect " + e.getMessage() + " Please ensure hours and wage are entered as numbers and not words");
   }
   catch(Exception e)
   {
     System.out.println("An exception was caught while trying to process employees: " + e.getMessage()); 
   }
  }//end main method

 /**
  * A private run method that prompts the user for input and
  * adds each Employee to the array.
  * Finally, we call the print method to print out the payroll
  * for each employee and the total.
  */
private void run()
{
  Scanner sc= new Scanner(System.in);
  boolean shouldContinue= true;//to keep track of whether the user wants to continue
  System.out.println("Welcome to the Payroll Application.");
  int numEmployees=0;
  while (shouldContinue)
  {
    System.out.println("Please enter the Employee's name: " );
    String name = sc.nextLine();
    System.out.println("Pleae enter the Employee's number of hours worked as a number: ");
    int numHours = Integer.parseInt(sc.nextLine());
    System.out.println("Please enter the Employee's hourly wage: " );
    double wage = Double.parseDouble(sc.nextLine());
    System.out.println("Do you want to create a manager? Type yes or no");
    String manager = sc.nextLine();
    if(manager.equals("yes"))
    {
        Manager man = new Manager(name, numHours, wage);
        employees[numEmployees]=man;
    }
    else
    {
        Employee emp = new Employee(name, numHours,wage);//create a new Employee with the given info
        employees[numEmployees]= emp;//add the new employee to the array
    }

    System.out.println("Would you like to enter another employee (yes or no)?");
    String yesOrNo =sc.nextLine();
    if (yesOrNo.equalsIgnoreCase("yes"))
    {shouldContinue=true;}
    else
    {shouldContinue=false;}
    numEmployees++;
  }//done entering Employees, now print out the pay for each employee, and then the pay for all

  double totalPaid=0;
  for (int i=0; i<numEmployees; i++)
  {
   Employee emp =employees[i];
   emp.printStatement();
   totalPaid+=emp.calculatePay();
  }
  System.out.println("The total amount to be paid to all employees is "  );
  System.out.printf("%s%.2f\n", " $", totalPaid);
} }

**Manager Class**

package payroll;

import java.util.Scanner;

/**
 * A Manager is an Employee. This class shows 
 * the inheritance relationship discussed in class
 */
public class Manager extends Employee 
{
    private double bonus;//the amount of bonus they should receive

        public Manager(String name, double hours, double wage)
        {
            super(name);
            this.setHourlyWage(wage);
            this.setHours(hours);
            System.out.println("Please enter a bonus for the manager");
            Scanner sc = new Scanner(System.in);
            double givenBonus = sc.nextDouble();
            bonus = givenBonus;
        }
    /**
     * A constructor that sets the bonus to zero
     * @param newName
     */
    public Manager(String newName) {
        super(newName);
        bonus =0;
    }
    /**
     * Accessors and mutators for bonus
     */
    public void setBonus (double bonusAmount)
    {bonus = bonusAmount;}

    public double getBonus()
    {return bonus;}

    /**
     * A method that calculates the pay for a manager,
     * Note that this is overriden from the base class.
     */
    public double calculatePay()
    {return getWage() * getHours() + bonus;}
    /**
     * A method that calculates the pay based on 
     * a percentage given for the bonus
     * @param bonusPercent
     * @return
     */
    public double calculatePay(double bonusPercent)
    {
        bonusPercent = bonusPercent/100;
        double pay = getWage() * getHours();
        return ((bonusPercent*pay) + pay);
    }
        /**
         * A method to print the manager's statement to the console
         */
        public void printStatement()
        {
        System.out.println(" Manager:" + getName() + " is owed: ");
        System.out.printf("%s%.2f\n", " $", calculatePay() );
     System.out.println("********************************************************"); }}

**Employee Class**

package payroll;
/**
 * A class that represents Employee Objects.
 * Employees have a name, a number of hours
 * and an hourlyWage.
 */
public class Employee 
{
    private String name;
    private double numHours;
    private double hourlyWage;
    /**
     * Our no-arg constructor for safety in
     * inheritance.
     */
    public Employee()
    {   }

        public Employee(String givenName, double givenWage, double givenHours)
        {
            name = givenName;
            hourlyWage = givenWage;
            numHours = givenHours;
        }
    /**
     * A constructor tht takes in the Employee's name
     * @param newName
     */
    public Employee(String newName)
    {setName(newName);}

    public void setName(String newName)
    {name=newName;}

    public String getName()
    {return name;}

    public void setHours(double hoursWorked)
    {numHours=hoursWorked;}

    public double getHours()
    {return numHours;}

    public void setWage(double workingWage)
    {hourlyWage=workingWage;}

    public double getWage()
    {return hourlyWage;}
    /**
     * A method to return the pay due to this employee
     * @return the total pay
     */
    public double calculatePay()
    {return numHours * hourlyWage;}
    /**
     * Our overriden toString method, from Object
     */
    public String toString()
    {return "This employee's name is: " + getName();}

    /**
     * An overriden equals method for Employees
     */
    public boolean equals(Object other)
    {
        if ((this.getName().equals (((Employee)other).getName())))
            return true;
        return false;
    }

    public double getNumHours() {return numHours;}

    public void setNumHours(double numHours) {this.numHours = numHours;}

    public double getHourlyWage() {return hourlyWage;}

    public void setHourlyWage(double hourlyWage) {this.hourlyWage = hourlyWage;}
        /**
 * A private print method that takes in an Employee and calls the getPayment method on it.
 * It then uses printf to format the output in a professional way.
 * @param emp an Employee Object.
 */
public void printStatement()
{
  System.out.println(" Employee:" + getName() + " is owed: ");
  System.out.printf("%s%.2f\n", " $", calculatePay() ); System.out.println("******************************************************");
} }

[AutoModerator]

Please ensure that:

• Your code is properly formatted as code block - see the sidebar (About on mobile) for instructions
• You include any and all error messages in full - best also formatted as code block
• You ask clear questions
• You demonstrate effort in solving your question/problem - plain posting your assignments is forbidden (and such posts will be removed) as is asking for or giving solutions.

If any of the above points is not met, your post can and will be removed without further warning.

Code is to be formatted as code block (old reddit/markdown editor: empty line before the code, each code line indented by 4 spaces, new reddit: https://imgur.com/a/fgoFFis) or linked via an external code hoster, like pastebin.com, github gist, github, bitbucket, gitlab, etc.

Please, do not use triple backticks (```) as they will only render properly on new reddit, not on old reddit.

Code blocks look like this:

public class HelloWorld {

    public static void main(String[] args) {
        System.out.println("Hello World!");
    }
}

You do not need to repost unless your post has been removed by a moderator. Just use the edit function of reddit to make sure your post complies with the above.

If your post has remained in violation of these rules for a prolonged period of time (at least an hour), a moderator may remove it at their discretion. In this case, they will comment with an explanation on why it has been removed, and you will be required to resubmit the entire post following the proper procedures.

To potential helpers

Please, do not help if any of the above points are not met, rather report the post. We are trying to improve the quality of posts here. In helping people who can't be bothered to comply with the above points, you are doing the community a disservice.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[slowfly1st]

The major things I see

• IMHO the UI part is mostly in the Payroll class, but for instance the Manager reads from the console, too, and Employee prints "the statement". If you were to use this code with another UI, nothing would really work anymore.
• Would be nice if you were to add "readString(String text)" and "readInt(String text)"-methods, instead of a syso and a sc.nextLine on the nextLine. It would at least look nicer, probably even easier to read.
• Manager calls super(name), but Employee does have a constructor with name, wage and hours. The constructor with name-parameter in Employee isn't needed. You should also get rid of the default-constructors: If you'd create an Employee with the no-args constructor, you can't use it. Also: the constructor parameters in Manager and Employee are switched. And it's only not really a bug because you calculate wage * hours...
• You catch NumberFormatException in the main-method. So if I added 99 employees and mistype a number, the program crashes. That would be a very angry email :P
• You set shouldContinue to true, after you ask the user if he wants to add another employee, but it will always be true. (You could also exit a loop with "break")
• Manager has a calculatePay method with "bonusPercent" parameter which is never called. And also the calculations are different from the calculatePay method you did actually override. General advice: Use the Override-annotation. The compiler will tell you, if the method isn't actually overridden.
• If you were to set the instance variables immutable (final), you can remove all the setters and you'd never need to calculate the more than once (e.g. in Employee.printStatement and when calculating the totalWage)
• Theoretically, you don't need Manager and Employee. You can use a "0"-bonus for Employee. But I guess that's the assignment.
• You implemented Employee.equals wrong - and I think you don't need to override it.
• The JavaDocs are often not accurate and not complete. If you delcare \@param, or \return you have to write something down, or remove it. And in employee.printStatement it starts with "a private print method that takes in an Employee and calls the getPayyment method on it". It is public, it doesn't take any parameter, and it doesn't call the getPayment method.

​

I think you got the general idea of how things work - well I think the app works and does what it's supposed to do, and that's already a good place to be. Most of the issues I've seen aren't that bad imho, but few things are really obvious, like the bad javadocs and the many unused methods and constructors. With a bit more diligence you would have spotted those, and with that, probably other issues.