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

New index.js for tiny js task #258

Merged
merged 8 commits into from
Aug 25, 2022
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions submissions/Levi123/tiny-JS/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/* Refer to https://github.com/OleksiyRudenko/a-tiny-JS-world for the task details
Complete the below for code reviewers' convenience:

Code repository: https://github.com/Levi123/a-tiny-JS-world
Web app: _put project's github pages URL here_
*/

// ======== OBJECTS DEFINITIONS ========
// Define your objects here
const dog = {
species: 'dog',
name: 'Rich',
gender: 'male',
legs: 4,
hands: 0,
saying: 'Hav-hav!',
friends: ['Bonya'],
}

const cat = {
species: 'cat',
name: 'Bonya',
gender: 'female',
legs: 4,
hands: 0,
saying: 'Mr-mr-mr',
friends: ['Rich', 'Catwoman'],
}

const female = {
species: 'human',
name: 'Olya',
gender: 'female',
legs: 4,
hands: 4,
saying: "Glory to Ukraine",
friends: ['Yura', 'Rich'],
}

const male = {
species: 'human',
name: 'Yura',
gender: 'male',
legs: 4,
hands: 4,
saying: "Glory to Ukraine",
friends: ['Olya', 'Bonya'],
}

const catWoman = {
species: 'human',
name: 'Catwoman',
gender: 'female',
legs: 4,
hands: 4,
saying: cat.saying,
friends: ['Bonya']
}
// ======== OUTPUT ========
/* Use print(message) for output.
Default tag for message is <pre>. Use print(message,'div') to change containing element tag.

Message can contain HTML markup. You may also tweak index.html and/or styles.css.
However, please, REFRAIN from improving visuals at least until your code is reviewed
so code reviewers might focus on a single file that is index.js.
*/

const prop = ['species', 'name', 'gender', 'legs', 'hands', 'saying', 'friends']

function printObjectNew(personToPrint){
const propToPrint = [];
prop.forEach((propOfObj) => {
propToPrint.push(personToPrint[propOfObj]);
})
Copy link
Member

Choose a reason for hiding this comment

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

A perfect use case for Array#map method.
Also prop is a global variable. Do not use global variables where there is no need, esp. if it is used in a specific function and only there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let newStr = propToPrint.join('; ');
print(newStr);
Copy link
Member

Choose a reason for hiding this comment

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

Think of converting property names into object's property values, then joining with '; '.
Make the method return the result. Currently it does two jobs: (1) convert data, (b) produce output, which doesn't really follow Separatoin of Concerns principles.

Use Array.map method to convert data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OleksiyRudenko did I understand correctly, I need to split this function into several?

Copy link
Member

Choose a reason for hiding this comment

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

You should use print inside this method. You can call print in a fragment of code where you want the output to happen in. And this function will do a single job - transformation of data. You will end up with a one-liner here.

Copy link
Member

@OleksiyRudenko OleksiyRudenko Aug 25, 2022

Choose a reason for hiding this comment

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

@Levi123 while looking through your updated code version I started wondering why is it so.

I am sorry, I apparently missed one very important word here. The comment above should read

You should not use print inside this method.

My apologies. We will fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Levi123переглядаючи вашу оновлену версію коду, я почав дивуватися, чому це так.

Вибачте, я, мабуть, пропустив тут одне дуже важливе слово. Коментар вище має прочитати

Ви не повинні використовувати всередині printцей метод.

Мої вибачення. Ми це виправимо.

i don't understand you

}

// function printObject(object){
// const prop = [];
// for (element in object){
// prop.push(object[element]);
// }
// let newStr = prop.join('; ');
// print(newStr);
// }

// printObject(catWoman);
// printObject(male);
// printObject(female);
// printObject(cat);
// printObject(dog);

printObjectNew(catWoman);
printObjectNew(male);
printObjectNew(female);
printObjectNew(cat);
printObjectNew(dog);
Copy link
Member

Choose a reason for hiding this comment

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

From the checklist above in the thread:

  • Code is DRY, which means that whenever you see a pattern in your code those should be eliminated as much as possible. Examples:
    • print(dog); print(cat); etc ... should be refactored employing Array.forEach as the least
    • `${obj.legs}; ${obj.name}; etc...` (yes, strings are also code) must be refactored employing appropriate Array methods

Requirements are always requirements. Double-check yourself against requirements until the principles helping to design a clean code become a habit.