Chantelle/Semret - C10 Edges - Adagrams #19
Conversation
…he commented out populate_letter_pool method
…ng into letter pool array and uses regex
AdagramsWhat We're Looking For
Hi y'all-- good work! The code is good and meets all the requirements! Overall, I found it extremely readable, with great names and a great approach to solve the problems. There are some small areas that I think could be simplified, and I'm making a comment on those, but nothing too big. Also, nicely done on the optional wave! Good work on this project! |
| random_draw = split_letter_pool.flatten.sample(10) | ||
| return random_draw | ||
|
|
||
| end |
There was a problem hiding this comment.
I think overall, your solution to this method is good-- it shows that you all know how to use iteration and specifically map to create an interesting array. My only concern is that I feel like maybe map forced you all to need to make a lot of arrays and a lot of loops-- I don't recommend any changes at this moment, but if you all ever take a minute to look at this problem with fresh eyes, there may be other approaches where it doesn't feel like "fighting" with map
| temp_hand = letter_in_hand.dup | ||
| inputted_word = [] | ||
| inputted_word << word.upcase.split("") | ||
| inputted_word.flatten! |
There was a problem hiding this comment.
Actually, calling word.upcase.split("") will return an array of letters split by each character...
aka, if word is "example", word.upcase.split("") will be ["E", "X", "A", "M", "P", "L", "E"]! You don't need to shovel this array into the empty array inputted_word so it's [["E", "X", "A", "M", "P", "L", "E"]] and then flatten it... you can just assign the result of split into inputted_word!
Therefore, you can turn the above code:
inputted_word = []
inputted_word << word.upcase.split("")
inputted_word.flatten!to
inputted_word = word.upcase.split("")Let me know if there are any questions on this
| check_letter = true | ||
|
|
||
| inputted_word.each do |letter| | ||
| index = 0 |
There was a problem hiding this comment.
Scope question: What is the smallest scope (the most specific/local block of code) that index needs to be in?
|
|
||
| if temp_hand.include?(letter) | ||
| index = temp_hand.index(letter) | ||
| temp_hand.delete_at(index) |
There was a problem hiding this comment.
I think that index doesn't get used until this if block, so you may not need to declare index = 0 before this if block. Try deleting the index = 0 above and run the tests to see if it still works!
| # creating a hash to store highest scoring word with its score value | ||
| highest_scoring_hash = { | ||
| :score => 0, | ||
| :word => words.first |
There was a problem hiding this comment.
You declare that the initial value of highest_scoring_hash has a value 0 for score and a value of the first word for word. You may want to make the initial value consistent and make the initial value of score equal to score_word(words.first)
| # when lengths are equal picking first (the one already stored in hash) | ||
| elsif current_word.length == highest_scoring_hash[:word].length || | ||
| highest_scoring_hash[:word].length == 10 | ||
| # do nothing |
There was a problem hiding this comment.
If you need to do nothing, then you probably don't need to have this elsif exist :)
|
|
||
|
|
||
| results = false | ||
| dictionary = CSV.read("assets/dictionary-english.csv",headers: true, header_converters: :symbol) |
Adagrams
Congratulations! You're submitting your assignment.
Comprehension Questions
Enumerablemixin? If so, where and why was it helpful?