Skip to content

fix(challenge): update Statistics Calculator: Step 27#53196

Merged
naomi-lgbt merged 61 commits intofreeCodeCamp:mainfrom
weilirs:multiple-issue
Apr 8, 2024
Merged

fix(challenge): update Statistics Calculator: Step 27#53196
naomi-lgbt merged 61 commits intofreeCodeCamp:mainfrom
weilirs:multiple-issue

Conversation

@weilirs
Copy link
Copy Markdown
Contributor

@weilirs weilirs commented Jan 15, 2024

Checklist:

Closes #52753

@github-actions github-actions bot added the scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. label Jan 15, 2024
@ghost
Copy link
Copy Markdown

ghost commented Jan 15, 2024

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Copy link
Copy Markdown
Contributor

@jdwilkin4 jdwilkin4 left a comment

Choose a reason for hiding this comment

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

As I am running this locally, and we will need to break this down further then I originally thought.

Here should be the correct flow for the first few steps.

Update Step 27

This step should focus on returning the array because we need a value to return if we are going to later call the getMode function and test it out.

updated description for step 27

To better understand how the `getMode` function is going to work, you will need to print out its contents. This will allow you to see what is happening as you build out the function. But first you will need to return the `array` so it can be tested properly.

Inside your `calculate` function return your `array` parameter.

Correct starting code for updated Step 27

const getMode = (array) => {
  const counts = {};

  --fcc-editable-region--
  
  --fcc-editable-region--
}

new Step 28

This step will be where you will need to call the getMode function and pass in numbers and log it to the console.

updated description for new step 28

# --description--

Inside the `calculate` function, you have already called the `getMean` and `getMedian` functions. 

Below those function calls, add a ` console.log(getMode(numbers))`.

updated starting code for new step 28

const calculate = () => {
  const value = document.querySelector("#numbers").value;
  const array = value.split(/,\s*/g);
  const numbers = array.map(el => Number(el)).filter(el => !isNaN(el));
  
  const mean = getMean(numbers);
  const median = getMedian(numbers);

  --fcc-editable-region--

  --fcc-editable-region--

  document.querySelector("#mean").textContent = mean;
  document.querySelector("#median").textContent = median;
}

new step 29

This step should be about seeing the results in the console

updated description for new step 29

# --description--

To test out your `getMode` function so far, call the `calculate` function. 

Then in your application, input the numbers `4`, `4`, `2`, `5` and click on the `Calculate` button.

Open up the console and you should see the array of numbers.

updated starting code for new step 29

const calculate = () => {
  const value = document.querySelector("#numbers").value;
  const array = value.split(/,\s*/g);
  const numbers = array.map(el => Number(el)).filter(el => !isNaN(el));
  
  const mean = getMean(numbers);
  const median = getMedian(numbers);

  document.querySelector("#mean").textContent = mean;
  document.querySelector("#median").textContent = median;
}

--fcc-editable-region--

--fcc-editable-region--

Hope that hopes

@gikf gikf added the js v9 cert This is for the JS V9 certification. label Jan 16, 2024
@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Jan 17, 2024

@jdwilkin4
Does step 27 look good now?

@jdwilkin4
Copy link
Copy Markdown
Contributor

Hey @weilirs !

As I was testing it locally, I noticed that the getMode function wasn't there.
It looks like in your last commit you accidentally removed the getMode function.

Screenshot 2024-01-17 at 10 01 50 AM

I pushed an update to your branch to resolve that. So you will need to pull down those new changes.

But also, while I was testing it I noticed a typo in my original instructions here

Inside your calculate function return your array parameter.

If you look at the calculate function, there is no parameter. There is a const variable called array but no parameter technically by that name. So to return an array parameter wouldn't work here.

The latest commit I pushed up resolves that and updates the starting seed code.
So now, you are good to go with moving on with the rest of the steps.

But also, while you are adding the steps, make sure you are running this frequently to make sure that the directions, seed code and tests all work and make sense in the context of the changes that you want to make.

Also, it is perfectly fine to push back on review suggestions if you notice an error while testing it out.
If you spot an inconsistency or error while testing that doesn't match with the suggestion, it is fine to bring it up and say "I was testing this out and I don't think suggestion is going to work for x,y,z reasons. We could try this instead."

Maintainers are always open to that feedback and happy to discuss new approaches because at the end of the day we all have the same goal. As long as the issue is fixed, it is totally fine if it is a different approach then what was originally listed on the issue. As long as we get to a good resolution, that is fine. 👍

@github-actions github-actions bot added the scope: i18n language translation/internationalization. Often combined with language type label label Jan 19, 2024
@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Jan 30, 2024

For the new step 29, what tests are needed since the user just types stuff into the input box and clicks a button.

@jdwilkin4
Copy link
Copy Markdown
Contributor

Based on the new description here for step 29, it looks like they still need to make a function call

--description--

To test out your getMode function so far, call the calculate function.

The tests should be for the function call

@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Jan 30, 2024

Based on the new description here for step 29, it looks like they still need to make a function call

--description--

To test out your getMode function so far, call the calculate function.

The tests should be for the function call

but if I call the calculate function like this
image

I got this Uncaught TypeError: Cannot read properties of null (reading 'value')
in the console which points me to the line 235 in the image below.

image

@jdwilkin4
Copy link
Copy Markdown
Contributor

After looking at the code, it looks like the calculate function is already called in the HTML in the form element.

so step 28 can include the console statement.

But even with that change, and testing it out with numbers, I see the array of numbers and the null message.

I replicated the project locally and on codesandbox and was not able to replicate the null error

https://63jzq6.csb.app/

Maybe there is something in this particular testing environment that is causing this error. 🤔

@naomi-lgbt Do you have any thoughts on how to resolve this issue?
For context, we are trying to break down step 27, to make it clearer for campers since a lot of people are confused on how to go about it.
But we are running into errors when trying to log to the console testing out the calculate function.

@jdwilkin4
Copy link
Copy Markdown
Contributor

Hey @weilirs !

It looks like changes were merged into main. That should resolve the issue you were running to earlier.
I would suggest pulling down the latest changes from main and resolve the merge conflicts to see if the issue persists

@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Feb 8, 2024

Hey @weilirs !

It looks like changes were merged into main. That should resolve the issue you were running to earlier. I would suggest pulling down the latest changes from main and resolve the merge conflicts to see if the issue persists

Seems like the same issue still persists
image

@jdwilkin4
Copy link
Copy Markdown
Contributor

You don't need to call the calculate function in step 29.

Remember that I had said this earlier last week.

After looking at the code, it looks like the calculate function is already called in the HTML in the form element.

So you don't need to call it again in step 29. Step 29 can just continue with building out that forEach.

I pulled down your branch and noticed that the changes in your frame.ts were not there.
That was where the issue was.

Looking at your branch history on GitHub it doesn't look like that update is in there.

Screenshot 2024-02-08 at 8 31 27 AM

Double check that your branch is up to date and double check the frame.ts file to make sure it has the correct changes made from this PR here
https://github.com/freeCodeCamp/freeCodeCamp/pull/53213/files

When I apply those changes locally and run step 28, the error goes away.
Screenshot 2024-02-08 at 8 35 02 AM

Hope that helps 👍

@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Feb 8, 2024

please take a look at the new step 29.

@jdwilkin4
Copy link
Copy Markdown
Contributor

The shorter version works for me.

@weilirs please apply those changes to step 26

@lasjorg
Copy link
Copy Markdown
Contributor

lasjorg commented Mar 22, 2024

@jdwilkin4 I wonder if we can omit the starting paragraph. It is repeating what was said in Step 25 and we are taking up a lot of vertical space already.

So maybe start it with

To calculate the occurrence of an element you can use an object with the following approach:
...same as before

Maybe we can also put the example object on one line?

{ 1: 3, 2: 3, 3: 3, 4: 3, 5: 2 }

@jdwilkin4
Copy link
Copy Markdown
Contributor

@lasjorg

It is repeating what was said in Step 25 and we are taking up a lot of vertical space already.

Yeah, if it is already mentioned in the previous step then it is fine to omit it in step 26.

Maybe we can also put the example object on one line?

Yeah, that's fine 👍

@weilirs Could make those changes, please and thank you 💯

weilirs and others added 5 commits March 25, 2024 14:22
…a-structures-22/learn-advanced-array-methods-by-building-a-statistics-calculator/65c4dc57418fd6bfc710d61d.md

Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
…a-structures-22/learn-advanced-array-methods-by-building-a-statistics-calculator/65ca2d0625aa3a3201067f70.md

Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
…a-structures-22/learn-advanced-array-methods-by-building-a-statistics-calculator/65e62efde0592ec4b4bb6a69.md

Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
…a-structures-22/learn-advanced-array-methods-by-building-a-statistics-calculator/65f83a7ca7047318e3ccff7c.md

Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
@lasjorg
Copy link
Copy Markdown
Contributor

lasjorg commented Mar 25, 2024

Also, just to be clear. The seed code does not assign 0 to counts[el]

It assigns counts[el] + 1 when counts[el] is a truthy value, or 0 + 1 when counts[el] is a falsy value.

At no point is counts[el] the value 0

const array = [2, 4, 4, 5];
const counts = {};

array.forEach((el) => {
  counts[el] = (counts[el] || 0) + 1
  console.log(counts[el]); // 1, 1, 2, 1
})

Just goes to show the solution code isn't the most readable.

@lasjorg
Copy link
Copy Markdown
Contributor

lasjorg commented Mar 26, 2024

I do have a little nitpick with step 29

Chaining forEach onto the returned array:

  1. It makes it look like forEach is an array method that returns an array.
  2. It ignores the empty line where the code should be written.
  3. It is not what the seed code looks like in step 30.

It isn't a huge issue but I would prefer if the code asked for to be written was the same as the seed code in the next step.

array.forEach(el => {

})
return array;

and not

return array.forEach((el) => {});

Maybe this instead?

Inside your `getMode` function, on the empty line above your return call `forEach` on `array`. Your `.forEach()` method should have an empty callback function that takes an `el` parameter.

In the next few steps, you will use this loop to count the frequency of occurrences of each number in the array.

@jdwilkin4
Copy link
Copy Markdown
Contributor

jdwilkin4 commented Apr 1, 2024

Hey @weilirs !

It looks like you one outstanding comment here
#53196 (comment)

If you could resolve that, then this should be good to go

Thanks 👍

@weilirs
Copy link
Copy Markdown
Contributor Author

weilirs commented Apr 1, 2024

Hey @weilirs !

It looks like you one outstanding comment here #53196 (comment)

If you could resolve that, then this should be good to go

Thanks 👍

I applied the changes.

Copy link
Copy Markdown
Contributor

@lasjorg lasjorg left a comment

Choose a reason for hiding this comment

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

LGTM.

It was a bit of work, but I think this part of the challenge is in a much better place because of it. So thank you for your contribution.

Copy link
Copy Markdown
Member

@naomi-lgbt naomi-lgbt left a comment

Choose a reason for hiding this comment

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

Why are we having them log it a second time? This seems like a great opportunity to reinforce the behaviour of function return values.

Co-authored-by: Naomi <nhcarrigan@gmail.com>
@jdwilkin4 jdwilkin4 requested a review from naomi-lgbt April 8, 2024 01:20
@naomi-lgbt naomi-lgbt merged commit a22e749 into freeCodeCamp:main Apr 8, 2024
jdwilkin4 added a commit to jdwilkin4/freeCodeCamp that referenced this pull request Apr 18, 2024
…53196)

Co-authored-by: jdwilkin4 <jwilkin4@hotmail.com>
Co-authored-by: Jessica Wilkins <67210629+jdwilkin4@users.noreply.github.com>
Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
Co-authored-by: Naomi <nhcarrigan@gmail.com>
ahmaxed pushed a commit to ahmaxed/freeCodeCamp that referenced this pull request Apr 25, 2024
…53196)

Co-authored-by: jdwilkin4 <jwilkin4@hotmail.com>
Co-authored-by: Jessica Wilkins <67210629+jdwilkin4@users.noreply.github.com>
Co-authored-by: Kolade Chris <65571316+Ksound22@users.noreply.github.com>
Co-authored-by: Naomi <nhcarrigan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

js v9 cert This is for the JS V9 certification. 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

Development

Successfully merging this pull request may close these issues.

Statistics Calculator: Step 27 (multiple issues)

7 participants