Branches - Xinran and Macaria #3

gracexinran · 2019-08-14T23:01:50Z

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature	Feedback
What are the components that make up a method?	First is the method signature, which is the top line; sometimes it contains parameters, and sometimes not. Then there is the method body which contains any code you want to execute in your method. Then end.
What are the advantages of using git when collaboratively working on one code base?	It has the benefits of containing multiple versions of your code, and the ability to open up the file for editing by a partner. It is a fast way to share your code and changes.
What kind of relationship did you and your pair have with the unit tests?	We began testing with them once we learned what they were. We tested for every feature added or edited.
Does your code use any methods from the `Enumerable` mixin? If so, where and why was it helpful?	No, we used each as we often created multiple variables within the each loop. However, we could have refactored to include an enumerable in order to create our array of indexes and to access it in our first method.
What was one method you and your pair used to debug code?	We used puts plenty in order to make sure that we were outputting/returning the values we intended. We used Rake to check against provided tests.
What are two discussion points that you and your pair discussed when giving/receiving feedback from each other that you would be willing to share?	It was excellent working with a partner with different strengths than me! Xinran is so good at figuring out syntax and bringing ideas to real life. Macaria is good at picking up on the main points of tasks and bringing up a lot of good ideas for our project.

tildeee · 2019-08-23T14:25:52Z

Adagrams

What We're Looking For

Feature	Feedback
General
Answered comprehension questions	x :)
Small commits with meaningful commit messages	x, I would probably prefer commit messages that are less "completes wave 2" and more "implements score_word method" or "refactors score table data structure" etc.
Code Requirements
`draw_letters` method	x
Uses appropriate data structure to store the letter distribution	x
All tests for `draw_letters` pass	x
`uses_available_letters?` method	x
All tests for `uses_available_letters?` pass	x
`score_word` method	x
Uses appropriate data structure to store the letter scores	x
All tests for `score_word` pass	x
`highest_score_from` method	x
Appropriately handles edge cases for tie-breaking logic	x
All tests for `highest_score_from` pass	x
Overall

Great work on this project, Xinran and Macaria! Your code is smooth, logical, readable, and has overall good code style.

There are a few opportunities for refactoring on this if you had more time on this project, so I've added a few comments.

That being said, your submission is great overall! Keep up the good work!

tildeee · 2019-08-23T14:26:13Z

lib/adagrams.rb

+    index_of_letters.delete(index)
+  end
+  return letters_in_hand
+end


If you had more time on this project to refactor, I think that there are some built in Ruby methods that could have made your solution a little bit shorter, such as shuffle (so you could randomly shuffle letters_arr and then delete off of letters_arr, instead of relying on index_of_letters so much.)

tildeee · 2019-08-23T14:26:19Z

lib/adagrams.rb

+end
+
+# This method checks that the letters_in_hand contains the plucked letters from previous method
+def uses_available_letters? (input, letters_in_hand)


Minor nitpick: You CAN define a method with a space between the method name and the input parens, but Ruby will give a warning, because it prefers it if the method signature looks more like def uses_available_letters?(input, letters_in_hand)

tildeee · 2019-08-23T14:26:41Z

lib/adagrams.rb

+  input.split('').each do |tile|
+    if letters_in_hand_copy.include?(tile)
+      index_of_tile = letters_in_hand_copy.index(tile)
+      letters_in_hand_copy.delete_at(index_of_tile)


Hm, in the above two lines, you find the index of tile and store it at index_of_tile, and then delete the element at that index of tile. Could we use the .delete method to delete the first occurance of tile's value? Replacing the above two lines with this line seems to work, and the tests still pass:

letters_in_hand_copy.delete(tile)

tildeee · 2019-08-23T14:26:49Z

lib/adagrams.rb

+      index_of_tile = letters_in_hand_copy.index(tile)
+      letters_in_hand_copy.delete_at(index_of_tile)
+    else
+      return false


It's clever to return false here, because this way the method exits as soon as you need to :)

tildeee · 2019-08-23T14:27:01Z

lib/adagrams.rb

+    end
+  end
+  shortest_word = highest_scores.min_by { |word| word.length }
+  return {word: shortest_word, score: words_scores.keys.max}


tildeee · 2019-08-23T14:27:14Z

lib/adagrams.rb

+end
+
+# OPTIONAL METHOD: Reads from a dictionary csv to determine if input is a valid word or not. To run wave-5-game.rb must access file from adagrams directory. 
+def is_in_english_dict?(input)


Fantastic work on the optional wave 5! This makes me happy!

tildeee · 2019-08-23T14:27:27Z

lib/adagrams.rb

+end
+
+# OPTIONAL METHOD: Reads from a dictionary csv to determine if input is a valid word or not. To run wave-5-game.rb must access file from adagrams directory. 
+def is_in_english_dict?(input)


Also... what about the tests? :)

tildeee · 2019-08-23T14:27:44Z

lib/adagrams.rb

+
+# OPTIONAL METHOD: Reads from a dictionary csv to determine if input is a valid word or not. To run wave-5-game.rb must access file from adagrams directory. 
+def is_in_english_dict?(input)
+  return DICTIONARY.include?([input.downcase]) ? true : false


Nice use of a ternary! Let's keep pushing this refactor ;) Consider:

return DICTIONARY.include?([input.downcase])

gracexinran added 5 commits August 12, 2019 16:09

wave-1

cbc136a

Remove class, add wave-2

bbe1b38

added wave-3 and wave-4

1e339b7

finished wave 5 includes old comments

0164a50

formatted

18e87c1

tildeee reviewed Aug 23, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Branches - Xinran and Macaria #3

Branches - Xinran and Macaria #3

gracexinran commented Aug 14, 2019

tildeee commented Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

tildeee Aug 23, 2019

Branches - Xinran and Macaria #3

Are you sure you want to change the base?

Branches - Xinran and Macaria #3

Conversation

gracexinran commented Aug 14, 2019

Adagrams

Comprehension Questions

tildeee commented Aug 23, 2019

Adagrams

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment