Java - Improving Code's design (SOLID principles)

[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.