Skip to content

Marian Horoiu - #1

Open
horoiu wants to merge 15 commits into
iQ-University:marian-horoiufrom
horoiu:marian-horoiu
Open

Marian Horoiu#1
horoiu wants to merge 15 commits into
iQ-University:marian-horoiufrom
horoiu:marian-horoiu

Conversation

@horoiu

@horoiu horoiu commented Mar 24, 2020

Copy link
Copy Markdown

Resolved all exercises. Each exercise has it's own commit.

@alexonaci alexonaci left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Very good, I liked that you used ES6 features, a lot of array methods and the code is clean

Comment thread index.js Outdated

var strArr = ["13", "2", "34", "14", "5", "86", "3.46"];

const typeCastAndAdd = arr => arr.map(item => (Number(item) + 2).toString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For this situation is okay to use Number(item). Don't forget that there is also the parseInt() method which can save you in some cases:
https://stackoverflow.com/questions/4564158/what-is-the-difference-between-parseintstring-and-numberstring-in-javascript#answer-4564199

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In this situation, if I use parseInt(), the last item "3.46" will be converted to an integer and will return an "5".
Maybe it would be good to use parseFloat() instead of parseInt() or Number() ??

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, sorry I missed the float. parseFloat() would be better ;)

Comment thread index.js
b = 8;

(function() {
[a, b] = [b, a];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

very nice that you used destructuring assignment

Comment thread index.js Outdated
const key = ["subject", "time", "teacher"];

const objClasses = classes.map(item =>
item.reduce((acc, val, ind) => ((acc[key[ind]] = val), acc), {})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you maybe make this a little more readable by naming the variables more descriptive?
but very good use of reduce, I like it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I did some updates to variable names and inserted some console.log's to highlight the current iteration elements and values, both in .map() and .reduce() methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants