TheOdinProject / javascript-exercises

MIT License
1.18k stars 30.27k forks source link

12_findTheOldest fix ambiguity and remove redundant test description #428

Closed donRehan closed 3 months ago

donRehan commented 4 months ago

fix ambiguity in test description , old was 'finds the person with the greatest age if someone is still living.' remove redundant test that checks for 'finds the person with the greatest age if the OLDEST is still living' since both this and the above test for ability to check if there is no death year field.

Because

I had faced difficulty understanding the tests so wanted to make it easier for others to understand the test when they are learning

This PR

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

fabulousgk commented 4 months ago

These tests actually check two seperate cases. In the first case it verifies that the function returns the oldest person when there is a person still living in the set. In the second case it makes sure that the function returns the oldest person when there is a person still living in the set AND that person is still living.

Why would you need both checks? Perhaps someone would write a function that simply skips living people completely and therefore misses a case. Or does not compute the living person correctly...etc.

donRehan commented 4 months ago

According to what I understand from you. We check for these two cases.

Oldest including living people. Oldest is the living person.

This means we want to check if the code in the two cases

doesn't Skip living people. doesn't Skip dead people.

Then I would like to change the comment to be as stated above. I will be adjusting it for better readability once I have your confirmation.

thatblindgeye commented 3 months ago

@donRehan just checking if you're able to make the updates as discussed in the above comment?

donRehan commented 3 months ago

@thatblindgeye

I apologize this is my first PR didn't know how to change it, but I believe I fixed it as per our discussion now.