OOP exercise

[kotlyar-andrey]

OOP exercise

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

[OleksiyRudenko]

@kotlyar-andrey very well done!
OOP is not really good for building hybrids. And this is one of the learnings from this exercise to take. The proper approach would be composition. You do not need to fix anything in these regards. Just reflect on possible flaws of OOP-like approach.

Still we need to add some consistency with regards to the business domain. Please check comments below.

[OleksiyRudenko]

Do we really want users to specify species for class Homo Sapiens?

[OleksiyRudenko]

Strictly speaking humans are animals as well. We need to resolve the matter in a consistent way.

[kotlyar-andrey]

Thanks for the review!
I've made changes, please re-review.

[OleksiyRudenko]

@kotlyar-andrey good fix. Yet we need to make this code compliant with OOP/SOLID principles.
You will find extra materials here helpful.
Feel free asking specific questions based on your learnings.

[OleksiyRudenko]

Do all animals have hands?
Imagine we popultae the world with birds, fish, rhinos and deer.
OOP helps to model the real world quite accurately avoiding defining properties and behavior that the counterparts in the real world do not have.

[kotlyar-andrey]

I think, I understand what you mean. Review, please)

[OleksiyRudenko]

@kotlyar-andrey very good.
Although there are two flaws that will make the project harder to maintain and scale if we want to expand it.

[OleksiyRudenko]

A class shouldn't know anything about external environment.
Using a side effect (print is a side effect) also makes the class less flexible.

[kotlyar-andrey]

Ok, I can rename display to getInfo and return value

getInfo() {
  const friends = this.getFriends();
  const saying = this.getSaying();
  return this._properties
    .map(prop => this[prop])
    .join("; ")
    .concat(`; ${saying}; ${friends}`);
}

End then print inhabitants

inhabitants.forEach(creature => {
   print(creature.getInfo());
});

[OleksiyRudenko]

Making class method just return some data is definitely a good design decision.

[OleksiyRudenko]

Not a really good approach to have a list of non-array props and handle array props in a totally different way. Besides, this method does two three things: converts array props to strings, builds object presentation string, prints the presentation string.

What if we have 20 more array props? What if we have props that keep objects?

We need a more consistent approach.

Suggest an approach before implementing changes in these regards.

[kotlyar-andrey]

Not a really good approach to have a list of non-array props and handle array props in a totally different way. Besides, this method does two three things: converts array props to strings, builds object presentation string, prints the presentation string.

We can refactor this method to three another function

What if we have 20 more array props? What if we have props that keep objects?

Every class has addProperties method for adding new properties.
If we want to get another data in child classes, we can override method getAnotherData

getDataProperties() {
   return this._properties.map(prop => this[prop]).join("; ");
 }

 getAnotherData() {
   const friends = this.getFriends();
   const saying = this.getSaying();
   return `; ${friends}; ${saying}`;
 }

 getInfo() {
   return this.getDataProperties() + this.getAnotherData();
 }

This is all what I can offer

[OleksiyRudenko]

Look into method overloading and accessing inherited method version.

class A {
  method() { handle class A props }
}

class B extends A {
  method() { handle class B props and call `super.method()` if we need anything from the base class }
}

[OleksiyRudenko]

@kotlyar-andrey great job done!
Takes notes of the comments below.

[OleksiyRudenko]

If some class (e.g. Creature) is extended by one and only one class (e.g. Animal) this means they can be effectively merged. Yes, we may have legless creatures in future, but that's exactly the point when we would split a base class and/or add yet another base class on the top of hierarchy tree.

[OleksiyRudenko]

We delegate child class property handling to a base class.
This is a legal approach. Although we should always consider first that every class handles its own and only its own properties. This way the handling logic will be encapsulated within the owning class and not spread across the hierarchy.