Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oop exercise #126

Merged
merged 8 commits into from
Aug 26, 2022
Merged

oop exercise #126

merged 8 commits into from
Aug 26, 2022

Conversation

OlStani
Copy link
Contributor

@OlStani OlStani commented Aug 11, 2022

OOP Exercise

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlStani nice start.

The task requires to print the species the inhabitants belong to.

Also check comments below and a DoD list by bot.

this.hands = hands
}
prepareToPrint() {
return [this.name, this.gender, this.legs, this.hands, this.saying].join(';')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is way to re-use parent class method. It makes sense as this helps to follow SOLID principles. Check the supplementary materials on the task's README.

Comment on lines 34 to 38
const dog = new Inhabitants('Bob', 'male', 4, 'woof!')
const cat = new Inhabitants('Kitty', 'female', 4, 'meow!')
const woman = new People('Sara', 'female', 2, 2, 'Hello!')
const catWoman = new People('Bella', 'female', 2, 2, `${cat.saying}`)
const man = new PeopleWithFriends('Mario', 'male', 2, 2, 'Hi!', ['Bob', 'Tom', 'Eva'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By convention class name should be in singular form unless it holds multiple entitites indeed. new People that returns single human reads a bit weird as there is no option to create multiple humans in a single call of new People.

No need to have People and PeopleWithFriends. This can be resolved by having default friends = [] in a class constructor signature and relevant logic in prepareToPrint method. Even if friends has no default value and assigned with none, it's value gonna be undefined, which is also quite handleable.

Imagine we want to specify neighbours, pets etc (and any such list may be empty or undefined). This should be handled by a single class.

@github-actions
Copy link

Thank you for contributing to this repo! ❤️️

A Tiny JS World -- OOP exercise check list

Relates to
A Tiny JS World OOP exercise.

Let's do some self-checks to fix most common issues and to make some improvements to the code while reviewers find some time to dedicate it to your submission.

Go through the checklist below, fix code as appropriate and mark fulfilled requirements when you genuinely believe you are done.

Please, feel free to ask questions here or seek for help in the Students' chat.

Check-list - definition of done

Minimal requirements to meet:

  • Implement a base class to inherit from
  • Employ default parameters
  • Each species is represented with its own class
  • No need to specify species at instantiation
  • Classes for species that do not have hands by natural design, do not consequently have hands or any equivalent property and do not inherit such properties from any base classes
  • All inhabitants are stored in a container (array or object)
  • JS native features are intensively employed (const, let, Array.map|join|forEach|..., etc)
  • Object methods like keys, values, entries as well as for...in shouldn't be used as these do not guarantee any particular order of keys/values
  • Properties used to form an object presentation string must be explicitly listed/specified; Use Array.map and Array.join to build a presentation string from the list.
  • A parent class shouldn't know anything about its children's props. Child classes must have good reasons to assign or refer to any parent's props. There are no such reasons in this task. Method overloading is to the rescue when building an inhabitant's presentation string.
  • OOP, SOLID and DRY principles are intensively employed

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:

Sincerely yours,
Submissions Kottachecker 😺

@OlStani
Copy link
Contributor Author

OlStani commented Aug 19, 2022

@OleksiyRudenko подивись, будь-ласка, коли буде час)

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlStani very well done!
Although species are missing.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 20, 2022

@OleksiyRudenko
які види відсутні? є класс для людей і для тварин, вони охоплюють всі властивості видів.

@OleksiyRudenko
Copy link
Member

Біологічні.

Cannot see any
image

@OleksiyRudenko
Copy link
Member

Please also check yourself against the checklist above and confirm explicilty you did and followed all listed requirements.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 21, 2022

@OleksiyRudenko
дійшло)
додав види і перевірив чекліст.

@OleksiyRudenko
Copy link
Member

@OleksiyRudenko
Copy link
Member

Does it work as intended end-to-end?
Check your code on your own. Do not expect a reviewer to "red-pen" every single tiny bug. Re-use your learnings from the past tasks.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 21, 2022

Види виправив.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 21, 2022

@OleksiyRudenko
Я перевіряв чек лист три рази. Я в третє в житті використовую js в завданні і не бачу помилок через брак досвіду, а не тому що хочу щоб їх знайшли за мене. Я вибачаюсь, що витрачаю Ваш час, але я просто не бачу що треба переробити.

@OleksiyRudenko
Copy link
Member

JS не має відношення до уважності і самоперевірки.

Дай будь-ласка відповіді на наступні запитання:
(1) Чого бракує у виводі демо?
(2) Звідки взялись ; в виразах на початку коду і чому вони не використовуються систематично по всій код базі? Питання не до ; в строкових літералах.
(3) Чого ти навчився на цій вправі і які підходи плануєш використовувати у майбутньому?

@OlStani
Copy link
Contributor Author

OlStani commented Aug 22, 2022

@OleksiyRudenko
1)Більш гарного вигляду і систематичної пунтуації. Додава пробіли та крапки з комами.
2)Видалив ";" на початку коду. Звертатиму на це більше уваги. Мені більше подобається коли їх не має, код виглядає чистішим.
3)Навчився наслідувати класи та їх методи. Мені важко зрозуміти масштабну необхідність класів, але мені здається що основна мета наслідування класів - створення нового коду , без порушень у старій частині. Наприклад якщо я захочу додати птахів я розштрю клас inhabitant, в той же час мені не доведеться писати всі властивості птахів, тому що частина вже написана в inhabitant, це зекономить час і зробить код коротшим і більш лаконічним.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlStani well improved.
A few improvement touches and we are done.

Comment on lines 17 to 22
this.hands = hands
this.friends = friends || []
}
prepareToPrint() {
return super.prepareToPrint() + '; ' + this.friends.join(', ')
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hands output is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

як ти це помічаеш?))

Comment on lines 15 to 16
constructor(specie, name, gender, legs, hands, saying, friends) {
super(specie, name, gender, legs, saying)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We assume all instances of Person are of human species.
We may let users skip passing this property and send a scalar string/constant to the base class' constructor.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 25, 2022

@OleksiyRudenko
подивись будь-ласка як матимеш час

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sejoker really close.
Go through construction chain to find a way to handle species.

Constructors are regular functions in terms of they handle parameters.

Comment on lines 15 to 19
constructor(name, gender, legs, saying, hands, friends) {
super(name, gender, legs, saying)
this.hands = hands
this.friends = friends || []
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may ... send a scalar string/constant to the base class' constructor.

What experession "sends" parameters to a super's constructor?

What is super and how did you come up with using it?

this.friends = friends || []
}
prepareToPrint() {
return 'human; ' + super.prepareToPrint() + [this.hands, this.friends.join(', ')].join('; ')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't need to hardcode species here. We have a base class to handle it. We just need to let it know what species we deal with.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 26, 2022

@OleksiyRudenko
це єдине що спало мені на думку)
слово super передає параметрb конструктора або методи від від наслідуваного класу до наслідуючого.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlStani nice try. Wrong direction though.
Make a step back. And think of what super(...) actually calls. How constructor is different from a function? Hint: not really. Do we have use variables received by function to send them to the next function we call or we can send anything we want/need?

class Animal extends Inhabitant {
constructor(specie, name, gender, legs, saying, tail) {
super(name, gender, legs, saying)
this.specie = specie
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have duplication of properties in class hierarchy that violates OOP/SOLID principles.
Shared properties must be owned by a base class and handled in one class.

@OlStani
Copy link
Contributor Author

OlStani commented Aug 26, 2022

@OleksiyRudenko
сподіваюсь так?)

@OleksiyRudenko
Copy link
Member

@OleksiyRudenko це єдине що спало мені на думку) слово super передає параметрb конструктора або методи від від наслідуваного класу до наслідуючого.

Не обов'язково тільки параметри наслідуваного класу. Йому можна згодувати що завгодно. Логічно, що якась кількість аргументів йому просто передається.

Copy link
Member

@OleksiyRudenko OleksiyRudenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlStani well done!

@OleksiyRudenko OleksiyRudenko merged commit af0d831 into kottans:main Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants