Oop classes #436

MarharytaBoichenko · 2022-09-04T20:34:50Z

OOP Exercise

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

er formatting and changes font-size to rem

MarharytaBoichenko · 2022-09-04T20:39:58Z

Sorry, need help, I can`t understand why I have such problem with this pull request and commits((
Where was the first mistake why I have all this files in my branch

github-actions · 2022-09-05T12:07:28Z

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. 😒

This PR contains irrelevant commits. You can see this under Commits and/or Files changed tab (you will find a couple of helpful tabs right under the PR title).

Most likely, you created a branch for this submission not from the main branch as instructed in section B122 of submission instructions.

Please fix and add a comment stating that you did this.

How to fix.

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 😺

OleksiyRudenko · 2022-09-05T12:08:08Z

Check the bot's suggestions and items 2 and 3 here.

We'd need to look into your fork's git history to find the exact point where things went wrong. Most likely you had the wrong order of merging (main should be merged in the feature branch, not vice versa) or committed directly to main on your fork.

…orks into main

MarharytaBoichenko · 2022-09-11T12:25:26Z

Thanks for great instruction! My problem was in committing to main branch directly, but it seems I did all this steps in the instruction to the wrong commit(
I took commit 8d092ec, the last in Popup, because in Dom task I committed to main but now I see all popup commits here(
It seems I can-t understand what commit I need for this step - " identify the most recent commit that both usptream and origin share (c4ebbab in our example)"

OleksiyRudenko · 2022-09-13T09:41:26Z

It seems I can-t understand what commit I need for this step - " identify the most recent commit that both usptream and origin share (c4ebbab in our example)"

You just look for a commit that is present on both upstream and fork histories. To see the upstream's history you'd need to clone it separately.

P.S. This PR looks good now. Thanks for fixing it.

github-actions · 2022-09-13T09:42:06Z

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 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. 😒

Please, make sure that your code follows the requirements based on the most common mistakes as well as both basic and advanced requirements from the original task.

Universal recommendations:

Make sure your code follows General Requirements
Use common sense or seek for an advice whenever requirements look ambiguous or unclear.

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

Sincerely yours,
Submissions Kottachecker 😺

MarharytaBoichenko · 2022-09-13T19:07:59Z

I checked my code according to the recomendations, please review

OleksiyRudenko

@MarharytaBoichenko good job!
I wonder why having suboptimal implementations that break OOP/SOLID principles?

OleksiyRudenko · 2022-09-20T17:28:43Z

submissions/MarharytaBoichenko/oop/index.js

+  toPrint() {
+    return this.properties
+      .map((prop) => {
+        const neededProp = this[prop] === undefined ? `no ${prop}` : this[prop];


Use Array#filter to keep props that make sense, then stringify what remains with Array#map.

OleksiyRudenko · 2022-09-20T17:30:00Z

submissions/MarharytaBoichenko/oop/index.js

+    this.legs = legs;
+    this.gender = gender;


What makes legs and gender different from name in terms of handling and generalization of shared properties?

I changed gender, but not legs, I think it differs because here class Animal extends Inhabitant in parents class Inhabitant there no legs.

Why legs do not deserve the same handling as name or gender?

I think my mistake is that I write legs for Animal but thought about legs as differnt properties , not as legs which Person has ). As I understand if it legs and this property is in Animal and in Person too so we need handle it as name and gender and legs will go from parent class. And if I decide that legs in class Animals is diffferent property - paws so in Animal it will be this.paw = paw . Is it right?

thanks, done

OleksiyRudenko · 2022-09-20T17:31:40Z

submissions/MarharytaBoichenko/oop/index.js

+    super(name, species, friends, saying);
+    this.legs = legs;
+    this.gender = gender;
+    this.properties = [...this.properties, "gender", "legs"];


A child class shouldn't directly change inherited property.
A base class shouldn't handle child's properties.
Altogether, every class should handle end-to-end the properties it owns immediately.
Use method overloading and explicit call to inherited method version.

Still an issue.
Use method overloading/overriding and super API to call inherited method version. super has other useful applications, not only to call parent's class constructor.

OleksiyRudenko

@MarharytaBoichenko great job!

OleksiyRudenko · 2022-09-26T20:36:09Z

submissions/MarharytaBoichenko/oop/index.js

+  toPrint() {
+    return super.toPrint();
+  }


No need to overload a method to only explicitly call an inherited version of the method.

OleksiyRudenko · 2022-09-26T20:39:37Z

submissions/MarharytaBoichenko/oop/index.js

+class Catwoman extends Cat {
+  constructor(name, paws, gender, hands, friends) {
+    super(name, "catwoman", paws, gender, friends);
+    this.hands = hands;
+  }
+
+  toPrint() {
+    return super.toPrint() + `, my hands: ${this.hands}`;
+  }
+}


Notice, how hybrids break OOP principles: it forces you to duplicate properties.
Although, there are other approaches to achieve this without code duplication, the major learning here should be that when you need to compose properties and behaviours in a non-strictly-hiearchical manner, OOP won't work, and composition pattern is a way to go. Keep in mind and when you face a need to build hybrids, read about and use composition.

MarharytaBoichenko added 6 commits August 5, 2022 00:44

Add popup files

95c73b7

Add changes to style files after review

cbc820a

Changes style files

ae25c71

Add changes in pretti

d37e29e

er formatting and changes font-size to rem

Fix focus on checkbox

cbfe0c5

Fix popup height

8d092ec

OleksiyRudenko added the irrelevant-commits Commits not related to current task label Sep 5, 2022

MarharytaBoichenko added 2 commits September 11, 2022 00:15

Merge branch 'main' of https://github.com/kottans/frontend-2022-homew…

d1d2ddc

…orks into main

Add classes for inhabitants

0084623

MarharytaBoichenko force-pushed the oop-classes branch from 2427b13 to 0084623 Compare September 11, 2022 11:58

OleksiyRudenko removed the irrelevant-commits Commits not related to current task label Sep 13, 2022

OleksiyRudenko added the task-TJSW-OOP label Sep 13, 2022

OleksiyRudenko added the self-check-done Student confirmed that self-checks against requirements/common-mistakes are done label Sep 20, 2022

OleksiyRudenko requested changes Sep 20, 2022

View reviewed changes

OleksiyRudenko self-assigned this Sep 20, 2022

MarharytaBoichenko added 2 commits September 20, 2022 23:28

Fix after review

2f4fa32

Set toPrint method to child classes

9109f46

MarharytaBoichenko requested a review from OleksiyRudenko September 26, 2022 20:06

OleksiyRudenko approved these changes Sep 26, 2022

View reviewed changes

OleksiyRudenko merged commit 3279480 into kottans:main Sep 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oop classes #436

Oop classes #436

MarharytaBoichenko commented Sep 4, 2022

MarharytaBoichenko commented Sep 4, 2022

github-actions bot commented Sep 5, 2022

OleksiyRudenko commented Sep 5, 2022 •

edited

Loading

MarharytaBoichenko commented Sep 11, 2022

OleksiyRudenko commented Sep 13, 2022 •

edited

Loading

github-actions bot commented Sep 13, 2022

MarharytaBoichenko commented Sep 13, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 20, 2022

MarharytaBoichenko Sep 20, 2022

OleksiyRudenko Sep 20, 2022

MarharytaBoichenko Sep 20, 2022

OleksiyRudenko Sep 20, 2022 •

edited

Loading

MarharytaBoichenko Sep 21, 2022

OleksiyRudenko Sep 26, 2022

MarharytaBoichenko Sep 26, 2022

OleksiyRudenko Sep 20, 2022

OleksiyRudenko Sep 26, 2022

OleksiyRudenko left a comment

OleksiyRudenko Sep 26, 2022

OleksiyRudenko Sep 26, 2022

Oop classes #436

Oop classes #436

Conversation

MarharytaBoichenko commented Sep 4, 2022

OOP Exercise

MarharytaBoichenko commented Sep 4, 2022

github-actions bot commented Sep 5, 2022

OleksiyRudenko commented Sep 5, 2022 • edited Loading

MarharytaBoichenko commented Sep 11, 2022

OleksiyRudenko commented Sep 13, 2022 • edited Loading

github-actions bot commented Sep 13, 2022

MarharytaBoichenko 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OleksiyRudenko Sep 20, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

OleksiyRudenko commented Sep 5, 2022 •

edited

Loading

OleksiyRudenko commented Sep 13, 2022 •

edited

Loading

OleksiyRudenko Sep 20, 2022 •

edited

Loading