OOP-exercise ready for review

[linkqwd]

OOP exercise Tiny js world

I'm asking only to check "composition" variant of code, other 2 versions I did just for the sake of practice.

Not sure if I met the requirements, still think that I understood it wrongly.

Anyway, I've tried to make my code as scalable and maintainable as I could.

ES6 composition code base | demo
ES5 inheritance code base
ES6 inheritance code base

[OleksiyRudenko]

@linkqwd You did a really excellent job. You have exposed both JS-specific OOP and general OOP mastery.

Solutions do fit requirements and could be approved right off.

However there are few comments below and requirements to fix. Please, deem those as a skills extension exercise you may opt out from. Please, let me know if you decide to opt out, and I shall then approve the PR as it is.

[OleksiyRudenko]

IDE signature hint would be more informative if function signature contained e.g. default parameter value.
You are not required to fix this.

[linkqwd]

sorry, don't understand. IDE signature, function signature - sounds confusing to me.
It means to set default parameters like this this.name = props.name || 'defaultName'; ??

[OleksiyRudenko]

You don't need to fix anything here. Just some ideas for you to consider.

Let's pretend your project contains tens or hundreds of modules/files.

So, when I as contributor want to instantiate new Inhabitant I want to know what parameters are.

One method is to go to the module that contains function Inhabitant... and look through code, another one is to enjoy function signature hinting from IDE. See the illustration below:

Hint gives me an idea of what arguments the function expects.

When argument expected is an object this though helps not much as IDE will hint me like this: Inhabitant(props) without disclosing what is props structure.

One method to solve the problem is to provide for a default value, e.g.

function Inhabitant(props = {
  species: 'human',
  name: 'Andy',
  sex: 'male',
  sound: 'Hi!',
}) {
 // ...
}

Now IDE will prompt default value I can use as a reference model.

Not all IDEs though offer this functionality. There are even better ways to annotate code.

One is JSDoc.
Having code annotated like

export default class Temperature extends Component {
  /**
   * Instantiate Temperature
   * @constructor
   * @param {HTMLElement} host
   * @param {{t: Number, unit: 'F'|'C'}} props
   */
  constructor(host, props) {
    super(host, props);
  }

IDE hinting works like this:

and this:

There are also other annotation methods, e.g. flow. The latter, though requires special processing and really suitable for tiny projects.

Hope the above helps.

[OleksiyRudenko]

Base (super) class is worth implementation and employment to design derived classes.
Please, fix.

[linkqwd]

I don't fully understand.
For me it sounds as a bad idea to mix classes (inheritance) and composition approach (or my solution not about composition?).
Anyway, I've tried, but haven't found how it could be implemented. Constructor in Class messes things up, and I don't know how to deal with it.
So I used straight object where I put methods and set prototype chain pointing on this object manually, for objects which were build from composition approach.

[OleksiyRudenko]

Base class is a concept. You can effectively mix the base class in just like any other (e.g. meower) just in front of derived class default properties to override base class defaults.

[OleksiyRudenko]

This function can effectively be OOP compliant method of a base class. Yet implemented as magic .toString.

[linkqwd]

Done.
But it was implemented not as a part of a base class, but as a part of object which was set up as prototype.

[OleksiyRudenko]

Let's assume method would expose all class properties to make code shorter.
Also specifying properties by names as strings is a really tricky thing. Imagine your code is split into modules and some contributor decides to rename .sex property as .gender.
Please, fix.

[linkqwd]

Sorry, don't understand this, but I rewrote this part of code (not familiar with modules)
This array 'thingsToShow' also allowed to control output order for properties, I thought that it was important.
Also each inhabitant could have function as it's own property (canBeFriendly() in this case), since I used object.assign - the problem is that function would be in output as well as other properties if I expose all properties

[OleksiyRudenko]

Ok then. Alternatively you could make this ordered list a part of class (yet another property). This way you encapsulate information about class within the class.

[linkqwd]

Thank you for reviewing and given guidelines.
I've tried my best, but feel like my solutions wasn't good enough.

I would like to dive in this topic more and do better job but then I won't be able to finish all other tasks in time.

p.s. sry for lots of text, it just I'm also practicing express myself in English at least somewhere, it seems important.

[OleksiyRudenko]

@linkqwd Thanks for fixing things. The remaining is rather for your consideration. So, just let me know if you're going to fix anything else. I will approve and merge otherwise (say, tomorrow morning).

[linkqwd]

@OleksiyRudenko thank you for advices, those were useful : )
Please, merge.

[OleksiyRudenko]

@linkqwd You did a great job worth special recognition.

Here is your achievement badge:

Please, provide me with a link to a document in your course repo so I could PR the badge code into.

Congratulations on the skills level-up!

[linkqwd]

@OleksiyRudenko
Thanks!
https://github.com/linkqwd/kottans_frontend/blob/master/README.md