Skip to content

Conversation

@quarkandgluon
Copy link

1) Correction on last test ('should return true if all pairs are guessed') so the correct implentation passes.

The original test was failing even with a correct implementation of the checkIfFinished method. This was due to two issues:

1.1. Issue with pairsGuessed and cards.length: The test was setting memoryGame.pairsGuessed = 8, but it wasn't setting memoryGame.cards.length. As a result, the condition this.pairsGuessed === this.cards.length / 2 in the checkIfFinished method was returning false, causing the test to fail even if the function is correctly implemented. This has been fixed by setting memoryGame.cards = new Array(16) in this spec of the test.

1.2. Issue with pairsClicked: The checkIfFinished method returns false if pairsClicked is 0. However, the test wasn't setting pairsClicked to a non-zero value (which will be the case when pairsGuessed are larger than 1) in the 'should return true if all pairs are guessed' spec, also causing the test to fail even if the function is correctly implemented.
This has been fixed by setting memoryGame.pairsClicked = 1 in that spec.

2) Correction of 'should return the shuffled (mixed) array of cards'

The test is not correctly implemented. It was simply checking if the returned array is different from the passed array, which could lead to false positives

The new test simulates a more realistic scenario by running the shuffle operation multiple times, and then checking if at least one card has changed its position in the array after the shuffle. This approach is more reliable and increases the likelihood of catching a faulty shuffle operation, as it's statistically improbable for a correctly implemented shuffle to maintain the original order of cards across multiple iterations.

@stale
Copy link

stale bot commented Oct 20, 2025

This pull request has been automatically marked as stale because it didn't have any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant