refactor(eight_ball): update responses for clarity and humor #873
Conversation
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
Contributor
Reviewer's GuideRefactors the eight_ball cog by replacing the single combined responses list with three categorical arrays (yes, no, unsure), populates them with updated, humorous/unfiltered strings to reduce bias, and adjusts the random selection logic to pick first a category then an entry. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey @electron271 - I've reviewed your changes - here's some feedback:
- The nested random.choice(random.choice([...])) is a bit obscure—consider first selecting a category (yes/no/unsure) and then picking a response for clearer intent.
- The long comma-separated string in unsure_responses ("Probably, Maybe, Possibly…") is treated as one response—split it into individual entries for better variety.
- A few of the added responses use strong profanity and might not suit all audiences—consider toning down or standardizing the humor level.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "Very doubtful", | ||
| "Why the hell are you asking me lmao", | ||
| "What???", | ||
| yes_responses = [ |
Contributor
There was a problem hiding this comment.
suggestion (performance): Recreating response lists on every call
Extract these lists to module-level constants to avoid rebuilding them on every call and improve readability.
| ] | ||
|
|
||
| choice = random.choice(responses) | ||
| choice = random.choice(random.choice([yes_responses, no_responses, unsure_responses])) |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider separating category selection and response selection into two steps for clarity, or flattening the lists if category distinction is unnecessary.
Suggested change
| choice = random.choice(random.choice([yes_responses, no_responses, unsure_responses])) | |
| # Instead of splitting into three separate lists and doing | |
| # choice = random.choice(random.choice([yes_responses, no_responses, unsure_responses])) | |
| # you can: | |
| # 1) keep your three lists (if you need them for other logic) | |
| # 2) pick a category in one step, then an element in another. | |
| # | |
| # This is clearer, avoids the hidden double‐call, and is functionally identical: | |
| categories = [yes_responses, no_responses, unsure_responses] | |
| category = random.choice(categories) | |
| choice = random.choice(category) |
If you don’t actually need to treat yes/no/unsure differently later, you can collapse back to a single list:
Suggested change
| choice = random.choice(random.choice([yes_responses, no_responses, unsure_responses])) | |
| # Flatten all responses and pick once: | |
| all_responses = [ | |
| *yes_responses, | |
| *no_responses, | |
| *unsure_responses, | |
| ] | |
| choice = random.choice(all_responses) |
Or, if you wanted a weighted pick across categories:
Suggested change
| choice = random.choice(random.choice([yes_responses, no_responses, unsure_responses])) | |
| # Use random.choices to weight categories explicitly | |
| categories = [yes_responses, no_responses, unsure_responses] | |
| # e.g. equal 1/3 chance per category: | |
| weights = [1, 1, 1] | |
| category = random.choices(categories, weights=weights, k=1)[0] | |
| choice = random.choice(category) |
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.
makes responses less biased and more humorous
Summary by Sourcery
Refactor the Magic 8 Ball command to use distinct yes/no/unsure response categories and refresh its phrasing with new humorous and varied messages.
Enhancements: