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.

Sign up for GitHub

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

Jump to bottom

feat(curriculum): add maps and sets workshop to FSD cert #57189

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

feat(curriculum): add maps and sets workshop to FSD cert #57189

wants to merge 15 commits into from
+3,570 −1

Conversation

Dario-DC
Copy link
Contributor

Checklist:

Closes freeCodeCamp/CurriculumExpansion#653

@Dario-DC Dario-DC added scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. full stack cert This label is for the new full stack certification labels Nov 16, 2024
@github-actions github-actions bot added platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: i18n language translation/internationalization. Often combined with language type label labels Nov 16, 2024
@Dario-DC Dario-DC marked this pull request as ready for review November 18, 2024 23:53
@Dario-DC Dario-DC requested a review from a team as a code owner November 18, 2024 23:53
@Dario-DC Dario-DC added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Nov 18, 2024
@a2937
Copy link
Member

a2937 commented Nov 20, 2024 •
edited
Loading

I see the finally blocks where some assignments in some of the tests. I'm a little confused by what the intention is by making these assignments. Can you help explain please?

Edit: I do understand in the cases where you switch out a function implementation and switch it back. But the others are losing me.

@Dario-DC
Copy link
Contributor Author

I see the finally blocks where some assignments in some of the tests. I'm a little confused by what the intention is by making these assignments. Can you help explain please?

Edit: I do understand in the cases where you switch out a function implementation and switch it back. But the others are losing me.

If I don't do that, following tests fail because sellPlants calls change the map. I could do that at the beginning of the following test, though.

Probably I should find a better way to test this. It seems a bit precarious.

@a2937
Copy link
Member

a2937 commented Nov 20, 2024

I like tests to be isolated when possible.

moT01
moT01 requested changes Nov 21, 2024
View reviewed changes
Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

Lotta fun @Dario-DC 🎉 Maybe we should rename this module to "plants, maps and sets" 😄 I left a number of comments to try and make things a bit more clear in some spots. They're just suggestions, feel free to leave them if you think it's fine.

.../english/25-front-end-development/workshop-plant-nursery-catalog/67349ba230cff237c0f2bdf2.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6734a2385c52c54d916a8b86.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/673339d96c56d36e963e888e.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6733b5632fb854bc6c58dc0d.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6733b5e6f210c1bd1afdf8b3.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/673762590f4bc1771c3a1e97.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/673521668be7905059b2d555.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6737a95d5b431860f96ca7bb.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6738ad2fd11775752d23ddc2.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6738ad2fd11775752d23ddc2.md

To complete this workshop, modify your `displayPlantsSet` to return a set with the unique plant common names in your catalog.

To achieve it, create an array containing the `commonName` of each plant object stored as a key of the `catalog` map. Pass the array to the `Set` constructor and return the set from the function.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To achieve it, create an array containing the `commonName` of each plant object stored as a key of the `catalog` map. Pass the array to the `Set` constructor and return the set from the function.
To achieve this, create an array of unique `commonName` values from your `catalog` keys. Pass the array to the `Set` constructor and return the set from the function.

I'm not sure if this is any better. Here's the solution I came up with:

code 
const displayPlantsSet = () => {
    const uniqueCommonNames = [];

    catalog.forEach((val, key) => {
        if (!uniqueCommonNames.includes(key.commonName)) {
            uniqueCommonNames.push(key.commonName)
        }
    })

    const catalogSet = new Set(uniqueCommonNames);
    return catalogSet
};

Could maybe mention to remove the add methods.

Copy link
Contributor Author

@Dario-DC Dario-DC Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The intention was to create an array with all the common names, without caring about repetitions. Then use the set constructor to get the unique values. Should I clarify that?
Anyway I'm checking just the final result.

zairahira
zairahira reviewed Nov 26, 2024
View reviewed changes
Copy link
Member

@zairahira zairahira left a comment

Choose a reason for hiding this comment

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

Had fun doing this workshop!
Left a few suggestions 👍️

.../english/25-front-end-development/workshop-plant-nursery-catalog/6734bbc0bd741598f942352e.md Outdated Show resolved Hide resolved
.../english/25-front-end-development/workshop-plant-nursery-catalog/6734c10a327760a665b7d5b9.md Outdated Show resolved Hide resolved
@moT01 moT01 self-assigned this Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zairahira zairahira zairahira left review comments

@a2937 a2937 a2937 left review comments

@moT01 moT01 moT01 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@moT01 moT01

Labels
full stack cert This label is for the new full stack certification platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. scope: i18n language translation/internationalization. Often combined with language type label status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Maps and Sets workshop to main repo
4 participants
@Dario-DC @a2937 @moT01 @zairahira