3 ms·
Some constructive criticism: abstract class Employee { // This needs to be implemented abstract calculatePay (): number; // This needs to be im
by cgrealy 7y ago
Some constructive criticism:
abstract class Employee {
// This needs to be implemented
abstract calculatePay (): number;
// This needs to be implemented
abstract reportHours (): number;
// let's assume THIS is going to be the
// same algorithm for each employee- it can
// be shared here.
protected save (): Promise<any> {
// common save algorithm
}
}
class HR extends Employee {
calculatePay (): number {
// implement own algorithm
}
reportHours (): number {
// implement own algorithm
}
}
class Accounting extends Employee {
calculatePay (): number {
// implement own algorithm
}
reportHours (): number {
// implement own algorithm
}
}
class IT extends Employee {
...
}
This is still a violation of SRP. IT does not care about the employees pay and shouldn't be forced to implement it.
Modern OO adds an additional rule to SOLID: favour composition over inheritance.
HR and IT should not extend from Employee. HR should be more like
class HR {
calculatePay (Employee e): number {
// implement own algorithm
}
reportHours (Employee e): number {
// implement own algorithm
}
}
The first question you should ask when deciding to use inheritance is: do I really need inheritance here?
Then, ask yourself again: Is <childClass> a <parentClass>?
In most businesses, HR is a Function or maybe a Department. HR might have Employees but HR itself is not an Employee.
- jpitz 7y agoThis is a big part of Liskov substitutability.
- Supermancho 7y agoEvery employee needs to be paid, even if that's 0. The original class was correct (a class is a namespace, then functionally dependent on language implementation). The implementation of calculatePay, is extensible as it stands. public Number calculatePay (Interface department) You delegate to the class (even if the interface is optional) Number val = defaultPayscale(); if(department) { val = department.getPay(); } return val; Because EVERYONE expects Employee to have a calculatePay of some sort, for accounting software. The idea that you want to smear a concept across domains for a misguided utopian design is over-engineering. Programmers have an average intelligence, so aim for supporting a simpler mentality, regardless of your beliefs.
- cgrealy 7y agoMy point is that IT doesn't (and shouldn't) need to know about employee pay. Extending the Employee class to HR, Accounting and IT is a bad design. You're violating ISP here. The Employee class can implement IThingThatGetsPaid, but IThingThatHasNetworkAccess should know nothing about pay. The Employee itself should have the minimal amount of information it needs. Prefer interacting with an object through a small API surface. It's not "smearing a concept across domains", it's the exact opposite. It's confining a concept to the appropriate domain. The original design pollutes the Employee interface with multiple concerns.
- jsymolon 7y ago> IT doesn't (and shouldn't) need to know about employee pay Project Budgeting, is one concern. You may not need to know the exact cost, but, having a hourly rate is useful. "A,B & C 80 hrs a piece, how much for the budget ?" Don't know have to ask HR for ....
- flukus 7y agoI don't know where you work but I generally find that employees do not calculate their own pay, apart from maybe reporting hours they tend to not be involved in the transaction at all, HR does the work and deposits it to the employees bank account. If we subscribe to the idea that OO should model the real world (I don't) then the HR class is definitely where the work should be done.
- Supermancho 7y ago> employees do not calculate their own pay My paychecks list the hours and rate paid, more often than not. But dispensing with that (because it's not relevant), a class isn't an actor-model. The tried-and-true Class Car->Drive() example (the car is doing something) is a contrived way to illustrate one of the ways a class can be used, as a learning tool. You don't want to create systems with that mentality, as it will always disappoint and spawn countless new webs of inheritance as time goes on. This is all hand waving, but that's a rather important issue to realize. There's no science behind design yet and it hasn't changed since the late 90s (when I started). My experience teaches me as with your own. I am afraid we'll just have to agree to disagree.
- jillesvangurp 7y agoI would augment that to "ALWAYS favor composition over inheritance" IMHO Solid is nice but I always struggle to remember what the acronym stands for. I mentally translate this to just optimizing two easy to remember metrics: classes should have low coupling and high cohesiveness. This also applies to functions and not just classes. E.g. functions with lots of parameters are a problem.