Skip to content

Exercise 12: Make solution less verbose #459

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

Merged
merged 8 commits into from
May 3, 2024

Conversation

nik-rev
Copy link
Contributor

@nik-rev nik-rev commented May 3, 2024

I felt like the solution is a little too verbose. I also wanted to take this moment to appreciate the ??= operator, I think learners will find it interesting at least :>

Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

I like this approach as a whole but personally I'd prefer it as a commented-out alternative solution, since there's absolutely nothing wrong with the current solution. A single alternative solution of relatively similar and reasonable complexity, I think is reasonable for the last exercise of the section.

Additionally, while defining getAge after findTheOldest works perfectly fine (because getAge is only actually called after it's defined) in the original, I agree it just reduces confusion to define it first. Could you also swap the order in the original solution?

Could you also amend this exercise's README 2nd hint to something like

This can be done using any variety of array methods, such as reduce, or even chaining multiple together.

nik-rev and others added 3 commits May 3, 2024 16:06
@nik-rev nik-rev requested a review from MaoShizhong May 3, 2024 15:19
nik-rev and others added 2 commits May 3, 2024 16:26
Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>
Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>
Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>
Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

🚀

@MaoShizhong MaoShizhong merged commit 1695abe into TheOdinProject:main May 3, 2024
Oussama5379 added a commit to Oussama5379/javascript-exercises that referenced this pull request Feb 1, 2025
* feat(12): improve solution by making it more cleaner

* refactor(12): rename `array` to `people`

* feat(12): improve explanation of `??=`

Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>

* feat(12): include both solutions

* feat(12): hint to use various array methods

* refactor(12): add semicolon

Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>

* feat(12): use block comments instead of line for readability

Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>

* refactor(12): remove unnecessary comment

Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>

---------

Co-authored-by: MaoShizhong <122839503+MaoShizhong@users.noreply.github.com>
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