fix: sort example had identical buggy and fixed functions - #204
Open
nareshAiNexus wants to merge 1 commit into
Open
nareshAiNexus wants to merge 1 commit into
nareshAiNexus wants to merge 1 commit into
Conversation
The 'bad' implementation used the same key as the fix: (-x['score'], x['name']). That means the test would pass before the fix was applied, defeating the whole point. Changed the buggy version to -x['score'] only (no tie-break), so duplicates keep input order instead of sorting alphabetically. That's the actual bug the example is trying to demonstrate. Also dropped the 'run 10 times' claim — Python's sorted() is stable so nothing is random. The real issue is order correctness, not non-determinism.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is broken
In Example 3 of the Goal-Driven Execution section, the buggy version and the fixed version of sort_scores are exactly the same:
Before (labeled buggy)
\\python
def sort_scores(scores):
return sorted(scores, key=lambda x: (-x['score'], x['name']))
\\
After (labeled fix)
\\python
def sort_scores(scores):
return sorted(scores, key=lambda x: (-x['score'], x['name']))
\\
The whole point of the example is to show the test failing first, then passing after the fix. That never happens here because both functions produce the same output.
The comment also says 'run 10 times, fails with inconsistent ordering'. That is not accurate. Python's sorted() is stable, nothing is random. The real problem is the order is input-dependent for ties, not that it is random.
What this PR fixes
Changed the buggy version to sort by score only, no tie-break:
\\python
def sort_scores(scores):
return sorted(scores, key=lambda x: -x['score'])
\\
Now the two versions are different. The test fails on the buggy version because Alice and Bob come out in input order instead of alphabetical. After the fix adds the name key, the test passes.
Also updated the verify comment to say what actually happens instead of claiming random failures.
Changes