Oop exercise #353

Tedzury · 2022-08-29T18:49:34Z

OOP exersice

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

github-actions · 2022-08-29T19:56:50Z

Hey!

Congratulations on your PR! 😎😎😎

Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code.

Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate.

If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat.

When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly.

Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒

A Tiny JS World -- OOP exercise check list

Relates to A Tiny JS World OOP exercise.

Check-list - definition of done

Minimal requirements to meet:

Optional level up (not required to implement):

Friends list is a list of objects refs rather than names (strings)
Cat-woman class is built employing composition rather than inheritance only

Bonus:

toString magic method; when implemented print(inhabitant) does the job as .toString is called implicitly
this.constructor.name; when used for output then no need to store species property

Helpful resources:

Universal recommendations:

Give variables and functions meaningful names. Avoid generic names like item, element, key, object, array or their variations. Exception: helper functions that are specifically and intentionally designed to be multipurpose.
Function names should start with a verb as they denote actions; variables are normally nouns; boolean variables/functions start with is, does, has etc; variable containing multiple entities and functions returning lists contain entity name in plural form.
Have consistent code style and formatting. Employ Prettier to do all dirty work for you.
Use common sense or seek for an advice whenever requirements look ambiguous or unclear.

Also take a note of the requirements above and follow them in all your future projects.

By the way, you may proceed to the next task before this one is reviewed and merged.

Sincerely yours,
Submissions Kottachecker 😺

Tedzury · 2022-08-30T05:33:28Z

Good morning. For the first view in self-check, everything seems to be alright. Humbly asking for review :)

stale · 2022-09-13T05:50:57Z

This issue has been automatically marked as stale because there were no activity during last 14 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

А. Чому так?
Найбільш розповсюджена причина: Студент не реагує на коментарі змінами коду і не задає запитань через брак часу або зміну життєвих пріоритетів. Покинуті піари відволікають менторів. Коли у студента з'явиться час, він/вона зможе перевідкрити той самий піар і продовжити роботу.

Б. Що робити, якщо в піарі нема оновлень, оскільки не зрозуміло, що треба зробити?
Варіант 1. Задати питання в самому PR.
Варіант 2. Задати питання в студентському чаті.

В. А якщо я все зробив(ла) і це ментор не рев'юває мої зміни?

Переконайся, що ти відреагував(ла) на всі коментарі або кодом, або запитаннями, або відповідями. Напиши в PR і в чаті, що чесно вважаєш, що все зроблено і попроси повторне рев'ю. Якщо щось не зрозуміло, задай запитання.
Реагуй на коментарі як менторів, так і інших учасників, включаючи ботів.
Не ігноруй прохання типу * "Let's do some self-checks ..." * "Go through the checklist below..." * "mark fulfilled requirements..." * "if you would silently ignore this recommendation, a mentor may think that you are still working on fixes"
навіть якщо вони написані ботом. Боти помічники і ментори покладаються на те, що прохання і пропозиції бота дотримуються.
Не лінись піти по лінках в коментарях, погуглити термінологію та скористатись Google Translate.
Можливо, у менторів склалися інші пріоритети через роботу, сімейні обставини і т.п. В такому разі, якщо ти зробив(ла) рекомендоване вище, то волай в чаті, що PR позначений stale, наче, все зроблено, а ментори чомусь не реагують - рятуйте!

Г. Хіба недостатньо того, що я додав(ла) коміт із змінами?
Часто буває так, що бачиш новий коміт, ідеш перевіряти, змін багато, доводиться перечитувати весь код. А потім з'ясовується, що одна невеличка зміна "відкладена на потім" чи з'являється ще один коміт і знов треба перечитувати все. Любіть нас, спілкуйтеся з нами - і ми відповімо повною взаємністю.

Традиційна пропозиція: задай питання по вищенаписаному в студентському чаті.

OleksiyRudenko

@Tedzury good job!
Yet implementation needs to follow OOP and SOLID principles.

OleksiyRudenko · 2022-09-13T09:17:00Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+      "species",
+      "name",
+      "gender",
+      "saying",
+      "friends",
+      "legs",


A base class should never handle child classes' properties. This breaks Encapsulation, Open-Closed and Separation of Concerns principles as any changes to child classes would require changes in one (or more) base class up the hierarchy.

Look into method overloading to have consistent interface and user experience where method implementation depends on class yet allows to re-use parent class' method for a similar purpose.

OleksiyRudenko · 2022-09-13T09:18:36Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+  constructor(name, gender, saying, friends) {
+    super(name, gender, saying, friends);
+    this.species = "human";
+    this.legs = 2;


legs looks like a shared property. Should be generalized to a base class just like any other shared property.
Same refers to species.

The criteria of generalization is property per se. It can bear different values for different class instances.

…them in mammal class

Tedzury · 2022-09-14T08:35:12Z

Good afternoon. Removed legs from base properties, added class Mammal and put legs to it.

Not sure that I get you right with this: "Look into method overloading to have consistent interface and user experience where method implementation depends on class yet allows to re-use parent class' method for a similar purpose."

I tried to implement that in such way, to prettify the printing output in needed order, added ternary operator into output func to modify the order of properties. Hope, it's what was needed.

OleksiyRudenko

@Tedzury quite a decent job.
Let's make it more OOP/SOLID compliant.
Method overloading and extended super API are to the rescue.

OleksiyRudenko · 2022-09-20T18:20:01Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+          "species",
+          "gender",
+          "legs",
+          "hands",


Every class should handle the props it owns immediately.
As a developer you do not want to touch base classes when adding a new one. This is what encapsualtion and open-closed principles from OOP/SOLID are about.

OleksiyRudenko · 2022-09-20T18:20:22Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+
+const tinyWorldInhabitants = [barbos, sonya, oleksii, victoria, anjela];
+
+tinyWorldInhabitants.forEach((item) => print(item.output()));


item is a generic term.

OleksiyRudenko · 2022-09-20T18:21:20Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+          "saying",
+          "friends",
+        ])
+      : (this.prettyOutput = this.propsArray);


Nested ternaries considered a bad practice. Can we make this fragment more obvious? KISS

Tedzury · 2022-09-22T20:39:59Z

Good night. Slightly refactored output function. Removed propsArray, applied function overrading to output( ) method and modified it into child classes.

OleksiyRudenko

@Tedzury well done.
Let's polish it using learnings from past tasks.

OleksiyRudenko · 2022-09-23T14:35:18Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+
+class Mammal extends Creature {
+  constructor(species, name, gender, saying, friends, legs) {
+    super(species, name, gender, saying, friends);git 


git here should have thrown an error. Didn't it?

OleksiyRudenko · 2022-09-23T14:39:23Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+  }
+
+  output() {
+    return `${this.name}; ${this.species}; ${this.gender}; ${this.saying}; ${this.friendsToString()}`;


More than 2 items. Array#join is still a thing.

Won't be a really good approach, but think of how destructuring could help you in more complicated cases:
const { propA, propC, propB } = this

Tedzury · 2022-09-24T07:06:41Z

Good day. Removed typo, apllied destructuring to properties in output( ) method.

OleksiyRudenko

@Tedzury let's optimize hierarchy.
Introducing classes just for the purpose of setting default values is an overkill.
I am not even sure we need classes for men and women. Or shy dogs and cats do not deserve sheDog/Cat and heDog/Cat classes?

OleksiyRudenko · 2022-09-26T17:05:31Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+  output() {
+    const { name, species, gender, saying } = this;
+
+    return `${name}; ${species}; ${gender}; ${saying}; ${this.friendsToString()}`;


Let's employ Array#join. Change this line only.
Remember, anything can be an array element. [ func() ] will add a result from func call as an array element.

OleksiyRudenko · 2022-09-26T17:07:48Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+class Mammal extends Creature {
+  constructor(species, name, gender, saying, friends, legs) {
+    super(species, name, gender, saying, friends);
+    this.legs = legs;
+  }
+  output() {
+    return `${super.output()}; ${this.legs}`;
+  }
+}


Mammal is the only class that extends Creature. Why do we need Mammal?
KISS

OleksiyRudenko · 2022-09-26T17:09:32Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+class Animal extends Mammal {
+  constructor(species, name, gender, saying, friends) {
+    super(species, name, gender, saying, friends, 4);
+  }
+}


Animal extends Mammal just to pass a specific value.
Solvable by setting default argument value.
KISS

Tedzury · 2022-09-26T19:52:36Z

Good night. Removed unneded animal class, man and woman. Just passing legs as default value, and gender to humans as an argument. Also implemented array.join in output method

OleksiyRudenko

@Tedzury nearly there

OleksiyRudenko · 2022-09-26T20:43:08Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+  output() {
+    const { name, species, gender, saying, friends } = this;
+
+    return `${name}; ${species}; ${gender}; ${saying}; ${friends.join(", ")}`;


There is still repetition ("; "). Pack whatever you need into an array and Array#join.
You will benefit a lot from recognizing use cases for Array methods.

OleksiyRudenko · 2022-09-26T20:46:03Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+  }
+}
+
+class Mammal extends Creature {


Mammal is the only class that extends Creature. Why do we need Mammal?

Any good reason for this?

Also, obviously there is no good reason to prefer 4 for legs. Decisions should be explainable.

Tedzury · 2022-09-27T06:58:20Z

Good morning. Left only base class and Human/Dog/Cat, legs are passed as default property in child classes. Implemented array with props to use array.join in the output method.

Sorry, but I feel very confused, I had exactly the same way of array.join in output two commits ago, but I undestood you in such way, that I shouldn't use it, so tried everything else, except manipulations with array :(

OleksiyRudenko

@Tedzury great job done!
Make sure to put some learnings on your diary.

OleksiyRudenko · 2022-09-29T20:28:40Z

submissions/Oleksii_Drohachov/OOP-exercise/index.js

+    this.saying = saying;
+    this.friends = friends;
+    this.legs = legs;
+    this.propsArray = [


Data type name as a part of variable name is not necessary (think if you switch to Map data type, you'd need to rename variable to reflect the change).
Plural noun is just sufficient.

Also props is quite specific term and kinda technical (as in e.g. "component props"). Here we mean "properties of inhabitant", just a normal word that in real life is never abbreviated. Do not save keystrokes on names, IDE will help you to autocomplete even German-language-style variable names.

Tedzury added 2 commits August 29, 2022 20:23

First commit. Sending for review

aed0edf

Second commit, removed hands property from initial list of properies

c30ef07

A-Ostrovnyy added the task-TJSW-OOP label Aug 29, 2022

stale bot added the 💤 Stale label Sep 13, 2022

OleksiyRudenko requested changes Sep 13, 2022

View reviewed changes

added base class mammal, removed legs from initial propsArray, added …

0c88212

…them in mammal class

stale bot removed the 💤 Stale label Sep 14, 2022

added function overloading handling

1a903c7

Tedzury requested a review from OleksiyRudenko September 14, 2022 08:41

OleksiyRudenko self-assigned this Sep 20, 2022

OleksiyRudenko requested changes Sep 20, 2022

View reviewed changes

refactoring, applying function overriding

780f4dc

Tedzury requested a review from OleksiyRudenko September 22, 2022 20:40

OleksiyRudenko requested changes Sep 23, 2022

View reviewed changes

removed typo, applied destructuring in the output method

a44323b

OleksiyRudenko requested changes Sep 26, 2022

View reviewed changes

removed animal, man and woman class

b0e9c76

Tedzury requested a review from OleksiyRudenko September 26, 2022 19:52

OleksiyRudenko requested changes Sep 26, 2022

View reviewed changes

removed creature class, implemented array for props

c5e3e05

Tedzury requested a review from OleksiyRudenko September 27, 2022 06:58

OleksiyRudenko approved these changes Sep 29, 2022

View reviewed changes

OleksiyRudenko merged commit 49f6fc4 into kottans:main Sep 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oop exercise #353

Oop exercise #353

Tedzury commented Aug 29, 2022

github-actions bot commented Aug 29, 2022

Tedzury commented Aug 30, 2022

stale bot commented Sep 13, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 13, 2022

OleksiyRudenko Sep 13, 2022

Tedzury commented Sep 14, 2022 •

edited

Loading

OleksiyRudenko left a comment

OleksiyRudenko Sep 20, 2022

OleksiyRudenko Sep 20, 2022

OleksiyRudenko Sep 20, 2022

Tedzury commented Sep 22, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 23, 2022

OleksiyRudenko Sep 23, 2022

Tedzury commented Sep 24, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 26, 2022

OleksiyRudenko Sep 26, 2022

OleksiyRudenko Sep 26, 2022

Tedzury commented Sep 26, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 26, 2022

OleksiyRudenko Sep 26, 2022

Tedzury commented Sep 27, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 29, 2022


		const tinyWorldInhabitants = [barbos, sonya, oleksii, victoria, anjela];

		tinyWorldInhabitants.forEach((item) => print(item.output()));

Oop exercise #353

Oop exercise #353

Conversation

Tedzury commented Aug 29, 2022

OOP exersice

github-actions bot commented Aug 29, 2022

A Tiny JS World -- OOP exercise check list

Check-list - definition of done

Tedzury commented Aug 30, 2022

stale bot commented Sep 13, 2022

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 14, 2022 • edited Loading

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 22, 2022

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 24, 2022

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 26, 2022

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 27, 2022

OleksiyRudenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tedzury commented Sep 14, 2022 •

edited

Loading