js oop exercise tiny world #219
Conversation
| this.saying = saying; | ||
| } | ||
| print() { | ||
| const specs = [this.species, this.name, this.gender, this.legs, this.hands, this.saying]; |
There was a problem hiding this comment.
Base class shouldn't know anything about child classes' internals.
This breaks SOLID principles.
| constructor(name, gender, saying) { | ||
| super(name, gender, saying); | ||
| this.legs = 4; | ||
| this.hands = 0; |
There was a problem hiding this comment.
A creature that doesn't have hands (or anything else) shouldn't have it. Not zero or explicitly undefined or null. Not prop at all. Imagine you add fish, birds, kangaroo. What a burden to keep track of all possible props. OOP helps to avoid this.
| class Human extends Inhabitant { | ||
| constructor(name, gender, saying) { | ||
| super(name, gender, saying); | ||
| this.species = 'human'; |
There was a problem hiding this comment.
species is a shared property of all inhabitants. It should be defined at some base class. This helps to keep code DRY and have any related in a single place.
| this.name = name; | ||
| this.gender = gender; | ||
| this.saying = saying; | ||
| this.props = [this.spacies, this.name, this.gender, this.saying]; |
There was a problem hiding this comment.
What will happen if the inhabitant's saying changes? Same refers to other props, saying is just the more likely to change.
| this.props = [this.spacies, this.name, this.gender, this.saying]; | ||
| } | ||
| print() { | ||
| print(this.props.join('; ')); |
There was a problem hiding this comment.
Base class shouldn't handle children's props. This breaks SOLID principles.
Use method overloading where appropirate.
| } | ||
| } | ||
|
|
||
| class CatWoman extends Animal { |
There was a problem hiding this comment.
From the task specs:
whenever you change cat's saying cat-woman's saying should change accordingly, they are strongly tied on some astral level
Will this happen under current implementation? How can we test this?
|
I hope now everything is okay. This topic some difficult for understanding(( |
There was a problem hiding this comment.
@DmytryjK nice fix.
Still need to work to make code SOLID/OOP compliant.
Consider reiterating through related learning materials.
Feel free asking questions.
| constructor (name, gender, saying, spacies, legs, hands) { | ||
| super(name, spacies, gender, saying); | ||
| this.spacies = 'human'; |
There was a problem hiding this comment.
(1) We receive spacies from the caller. We send it to the super class. We assign it directly. Something is not required.
(2) If we hardcode species here. WHy do we need class user to specify any at all? Why not to send the hardcoded value to the super class constructor, as it belongs to it, and let the super class handle it as they like it?
| this.spacies = 'human'; | ||
| this.legs = 2; | ||
| this.hands = 2; | ||
| this.property.push('legs', 'hands'); |
There was a problem hiding this comment.
We do not want a superclass to handle child class properties.
This breaks Encapsulation principle of OOP and Open-Closed principle of SOLID.
The language has tools to handle this situatuion.
Use method overloading and opportunity to call inherited method.
There was a problem hiding this comment.
@DmytryjK great!
Now the final touch. Looks like legs is something that all inhabitants have.
No good reason not to generalize this property and its handling to the base class.
This will be OOP/SOLID compliant and also the code will be DRYer.
There was a problem hiding this comment.
@DmytryjK you did a great job!
Hierarchical classes are perfect.
Hybrid is questionable, but this was intentional. Check the comments below.
Anyway you did the great job with the hybrid, considering the limitations of OOP/SOLID principles.
| get props() { | ||
| return super.props; | ||
| } |
There was a problem hiding this comment.
As long as child class doesn't add any new behaviour to the overloaded method, we do not need to overload it. It is inherited anyway.
You do not need to fix this. Take the learning. Let's consider this a stub for next iteration where we probably would want to add tails, whiskers or retractable claws.
| super(name, gender, cat.saying); | ||
| this.spacies = 'catWoman'; | ||
| this.legs = 2; | ||
| this.hands = 2; | ||
| } |
There was a problem hiding this comment.
Looks like CatWoman has more in common with humans than with cats.
Also note that we had to "re-add" hands and overwrite legs.
Actually OOP is not good with hybrids. The proper pattern for hybrids is composition.
You will have a chance to study composition patterns.
For now we done as much as possible in these regards with OOP.
Take this learning. Also check class static methods and properties. This is yet another way to "borrow" behaviour from another class.
OOP Exercise Tiny JS
Demo |
Code base
The code is submitted in a dedicated feature branch.
Only code files are submitted.
Please, review.