Update of rotate_pairs #70
Open
Conversation
This file contains 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
The current solution does not check for all rotated words, due to the stop argument of the range being 14 instead of 26 in the rotate_pairs function. For example, the word "act" should print out "act 24 yar", but currently does not print out anything.
As the current solution uses a somewhat brute force approach, my suggested solution uses a different approach, where each word is rotated only once. It also makes use of a dictionary to store the rotate pairs, by using the setdefault method covered in Exercise 11.2.
This suggested solution also directly addresses the specification of the exercise description ("Write a program that reads a wordlist and finds all the rotate pairs."). Only the word_list needs to be provided as argument, and a list containing lists with all rotate pairs is returned. Each word is thus contained only once in the returned list of lists.
Since the suggested approach does not need to check for the presence of a word in the list of words, the words can be provided simply as a list instead of a dict.
Update rotate_pairs.py
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.
Addresses issue #69
The current version of the rotate_pairs function does not check for all rotated words, due to the stop argument of the range being 14 instead of 26. For example, the word "act" should print out "act 24 yar", but currently does not print out anything.
Replacing 14 by 26 would fix this.
However, as the current solution proposes a somewhat brute force approach, my suggested solution uses a different approach, where each word is rotated only once. It also makes use of a dictionary to store the rotate pairs, by using the setdefault method covered in Exercise 11.2.
This suggested solution directly addresses the specification of the exercise description ("Write a program that reads a wordlist and finds all the rotate pairs."). Only the word_list needs to be provided as argument, and a list containing lists with all rotate pairs is returned. Each word is thus contained only once in the returned list of lists.
Since my suggested approach does not need to check for the presence of a word in the list of words, the words can be provided simply as a list instead of a dict.
Feel free to edit my suggested solution in any way you'd like.