freeCodeCamp icon indicating copy to clipboard operation
freeCodeCamp copied to clipboard

Advanced Dice Game - Step 4: Incorrect code is passing tests

Open dinahess opened this issue 9 months ago • 5 comments

Describe the Issue

Referring to this forum post (https://forum.freecodecamp.org/t/review-algorithmic-thinking-by-building-a-dice-game-step-4/741008), a student added the if statement to check if a user had rolled 3 times after the call to rollDice(), but it passed the test when rollDiceBtn.disabled = true; was added after the alert();

Affected Page

https://www.freecodecamp.org/learn/javascript-algorithms-and-data-structures-v8/review-algorithmic-thinking-by-building-a-dice-game/step-4

Steps to Reproduce

Navigate to Step 4 of the Advanced Dice Game in the JavaScript Algorithms and Data Structures curriculum and check this code, which passes the tests and should not:

rollDiceBtn.addEventListener("click", () => {
  rollDice();
  rolls++;
  if(rolls === 3){
    alert("You already rolled 3 times. You must select a score now.");
    rollDiceBtn.disabled = true;
  } 
});

Now try this code, which fails the tests (as it should):

rollDiceBtn.addEventListener("click", () => {
  rollDice();
  rolls++;
  if(rolls === 3){
    alert("You already rolled 3 times. You must select a score now.");
  } 
});

Recommended fix or suggestions

Fix the tests for this step to fail code that does not have the if statement to check for the maximum 3 rolls as the first statement in the event listener.

Screenshots

No response

dinahess avatar Mar 26 '25 17:03 dinahess

This is a bit convoluted, but core behavior for the version with rollDiceBtn.disabled = true; is correct - only 3 rolls will be allowed and alert is displayed after the 3rd roll. Meanwhile version without it permits further rolls.

Version 1:

  • Roll 1
  • Roll 2
  • Roll 3 - displays alert, disables button
  • No further rolls possible

Version 2:

  • Roll 1
  • Roll 2
  • Roll 3 - displays alert
  • Roll 4 - no errors
  • ...

gikf avatar Mar 26 '25 19:03 gikf

So, in the "Steps to Reproduce," both versions of the code are incorrect because if(rolls === 3){...} should come before the call to rollDice(), right? I'm not following why you think the version of the code that disables the button is correct.

dinahess avatar Mar 26 '25 21:03 dinahess

So, in the "Steps to Reproduce," both versions of the code are incorrect because if(rolls === 3){...} should come before the call to rollDice(), right?

Not necessarily, as it can be observed in the first case. What should be where in the callback function is implementation detail, but there isn't specific implementation that's checked. Tested is the expected behavior, which first code fulfills. In a bit strange and convoluted manner, but it does.

Does it display alert after third roll? Yes. Does it prevent from rolling more than three times? Yes.

gikf avatar Mar 27 '25 05:03 gikf

I see it now. Thank you for your patience. I'll get back to the OP on the forum post I cited and correct my misunderstanding.

dinahess avatar Mar 27 '25 22:03 dinahess

Subject: Fix: Step 4 Dice Game Test Incorrectly Validates Button Disable Order

Issue: The test for Step 4 of the "Algorithmic Thinking by Building a Dice Game" incorrectly passes code that disables rollDiceBtn after the alert(). This allows unintended behavior where users can click the button again during the alert pause.

Current Broken Test Behavior:

Passes (but shouldn’t) if (rolls === 3) { alert("..."); // Alert blocks execution rollDiceBtn.disabled = true; // Too late! }

Solution: Modify the test to enforce:

Early Check: Validate rolls === 3 before rollDice().

Disable First: Ensure disabled = true runs before alert().

Fixed Test Logic (Pseudocode):

Correct validation logic: assert( userCode.includes('if (rolls === 3)') && userCode.indexOf('disabled = true') < userCode.indexOf('alert(') && !userCode.includes('rollDice()', userCode.indexOf('if (rolls === 3)')) );

Repro Steps:

Submit [this incorrect code](https://forum.freecodecamp.org/t/review-algorithmic-thinking-by-building-a-dice-game-step-4/741008) (passes).

Submit [properly ordered code](https://replit.com/@YourExample) (fails).

Impact: Users learn flawed event-handling logic (race condition risk).

Let me know if you’d like a PR for this! Why This Works:

Precision: Targets the exact test flaw (order of operations).

Actionable: Provides pseudocode for the fix.

Context: Links to forum discussion for urgency.

Abdallah-scout avatar Apr 02 '25 12:04 Abdallah-scout

It doesn't look a change will be needed here. So I am going to close this issue. If you feel like the tests should be updated, then feel free to reopen this issue on more details for updates to the tests

jdwilkin4 avatar Apr 12 '25 20:04 jdwilkin4