8000 12_findTheOldest: Clarify test descriptions by RyanMcEntire · Pull Request #340 · TheOdinProject/javascript-exercises · GitHub
[go: up one dir, main page]
Skip to content

12_findTheOldest: Clarify test descriptions #340

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
Merged
merged 1 commit into from
Jul 27, 2023
Merged

12_findTheOldest: Clarify test descriptions #340

merged 1 commit into from
Jul 27, 2023

Conversation

RyanMcEntire
Copy link
Contributor

Because

The tests for 12_findTheOldest lead to frequent confusion because of the disconnect between the English sentence in the test description, and the expected outcome of the test.

The confusion comes from expectations of how we use English. Culturally in English speaking countries (i can't speak for others), we are not used to referring to anyone who is already dead as the oldest. When 2 people are dead, and someone is alive and 17 years old, that person is generally thought about as "the oldest", because oldest is reserved for the living.

Culturally, the second test: 'finds the person with the greatest age if someone is still living' would be considered ambiguous English. That's the test that causes the most confusion.

Additionally, the test passing arguments are too similar to the function itself.

The findTheOldest() pass condition is 'finds the oldest person!'

To make this less ambiguous and less tautological, I suggest changing
'the oldest person'
to
the person with the greatest age

I believe this helps with some of the common tendency to infer that the person who is living is older than the person who is dead, no matter how old the dead person was when they were alive.

This PR

  • Made changes to all three test descriptions to replace oldest person to person with the greatest age.

Issue

Closes #XXXXX

Additional Information

Link to a recent discussion where many people expressed confusion while solving this exercise.
Here are some more examples of people getting caught up on understanding the wording.

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the location of change: brief description of change format, e.g. 01_helloWorld: Update test cases
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If this PR includes changes that needs to be updated on the solutions branch, I have created another PR (and linked it to this PR).

@WongYC-66
Copy link

Not sure how to suggest a solution, apologize this reply wouldn't be related to this pull request.

Is this better solution?

const findTheOldest = function(people) {
    let orderd = people.map(x => {
        x.age = (x.yearOfDeath || new Date().getFullYear()) - x.yearOfBirth;
        return x;
    })
    orderd.sort((a,b) => b.age - a.age)
    return orderd.shift()
};

@CouchofTomato CouchofTomato requested a review from 01zulfi July 24, 2023 15:48
01zulfi
01zulfi approved these changes Jul 27, 2023
View reviewed changes
Copy link
Member
@01zulfi 01zulfi left a comment

Choose a reason for hiding this comment

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

lgtm 🚀

@01zulfi 01zulfi merged commit 191a43a into TheOdinProject:main Jul 27, 2023
Oussama5379 added a commit to Oussama5379/javascript-exercises that referenced this pull request Feb 1, 2025
@Oussama5379
Merge pull request TheOdinProject#340 from RyanMcEntire/getting_older…
cd5c581 
…_test_wording

12_findTheOldest: Clarify test descriptions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@01zulfi 01zulfi 01zulfi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RyanMcEntire @WongYC-66 @01zulfi
0