Fixed issue with certain types not in sets - #150
Conversation
…otherwise it loops until it does
|
i just commented out the previous code, will be cleaned up after you test it out |
|
|
There was a problem hiding this comment.
General Feedback:
Without testing it myself, the logic looks sound overall.
- Please avoid commenting out old code. Instead of replacing the existing code entirely, write your new logic around it. Commenting out old code can clutter the file and make the git history harder to follow. It's better to preserve the original code and build upon it if necessary.
- Complete the docstrings when updating them. If you're updating docstrings, make sure to finish them properly. Also, it’s best to focus on one task at a time. Changing things across the entire file (e.g., docstrings, code, formatting) can create difficulties when merging later, especially if multiple changes are spread throughout the file.
Because of the commented-out code, I'm rejecting this PR for now. Please revise accordingly and resubmit.
| # result = [random.choice(items) for items in category_dict.values()] | ||
|
|
||
| # return result | ||
|
|
There was a problem hiding this comment.
Why is everything above this line commented out?
If it's for experimentation, the whole point of using git is to track different versions and iterations of your code. You don't need to comment things out; git allows you to revert to previous versions and manage isolated changes. Commenting out code can also make it harder to understand the code's history.
Please avoid submitting files with commented-out old code.
Most teams or projects will reject files with unnecessary commented code, as it clutters the codebase.
Don't replace code by commenting it out and pasting in a copy.
Doing so will obscure the git history of those lines, making it hard to see who contributed them. While this may be an oversight, it could be seen as problematic or even malicious by other developers. You're essentially overwriting their attribution.
There was a problem hiding this comment.
my appologies, still getting used to using git and having multiple branches. noted for next time
| results = Backend.query_api(self, query) | ||
| # Check if the selected type is available within the selected set | ||
| query = Backend.construct_query(self, randomSelect) | ||
| print("Query: " + query) |
There was a problem hiding this comment.
Remove. No need to print to console except when debugging. We can remove this.
| randomChoice.append(random.choice(processed_cards)) | ||
| # If no cards found, the loop will repeat to try a new random selection | ||
| if not processed_cards: | ||
| print("No cards found for this set/type combination. Retrying...") |
There was a problem hiding this comment.
Remove. No need to print to console except for debugging. Not for final code. If wanting to print a meaningful message that is more error handling directed you should use try catch statements that form a meaningful exception object.
There was a problem hiding this comment.
will remove comments and re-submit branch. those were left so you could test for yourself
|
i dont know if i need to delete the other commit, but i updated it with the requested changes |
|
only thing i am unsure of is the un-finished doc strings? i updated it with saying it will loop until a valid card is found? do i need more info |
2 participants
added a loop to check if the randomized function provided valid data otherwise it loops until it does