Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ports- cloudy and mina #2

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

CloudyLopez
Copy link

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? name, parameters, dependent on if explicit return of value.
What are the advantages of using git when collaboratively working on one code base? track changes and pushing changes to access easier across multiple computers
What kind of relationship did you and your pair have with the unit tests? not a good one at first, once we realized HOW to implement them, it was ok.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? yes, we used .inject it was helpful when summing word score.
What was one method you and your pair used to debug code? we used the rake and the debugger.
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? open communication and a good attitude.

@tildeee
Copy link

tildeee commented Mar 3, 2019

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions x
Both teammates contributed to the codebase x
Small commits with meaningful commit messages x, we'll be looking for commit messages that aren't "wave 2 complete" etc, but more about what kinds of changes did you make "implemented logic to score a word" 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

Good work on this project, Cloudy and Mina!

You did a great job on getting working implementation in a readable, logical, clean way. I really liked some of your solutions; especially the ones with Enumerable methods, and with clear conditionals like the highest_score_from method!

I left a few comments, one mostly about one not-very-concise piece of code.

That being said, good work on this overall!

puts "Please make the best work with the letters you were dealt."
new_word = gets.chomp.to_s.upcase

uses_available_letters?(new_word, drawn_letters)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, feel free to delete any unused files before submitting!

my_letters = []
random_letter = pool_of_letters.shuffle
10.times do
my_letters.push(random_letter[-1])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it clear what's going on: You have the pool_of_letters array, and then shuffle it, and put the shuffled array into a variable called random_letter. Then, you do this ten times: you push into your array my_letters the value of the item that is last in the random_letter array (the thing at index of -1 is the last item). And then you call random_letter.pop, which removes the last item from the array. This strategy is great! It just feels a little unreadable. I think that the name random_letter feels inaccurate, since it doesn't represent one single random letter, but the array of the shuffled pool of letters. Also, instead of maybe using the syntax random_letter[-1], even though it's very short and concise, it might be more readable to use random_letter.last.

There are also interesting things that .pop does that you'll learn about in the future :)

end

def uses_available_letters?(input, letters_in_hand)
word = input.upcase.split(//)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just remembered that I was supposed to get in touch with Cloudy so that her VS Code indented with 2 spaces instead of 4 :)

my_score += 8
end

return my_score
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice method implementation, and nice use of Enumerable methods!

elsif word.length < winning_hash[:word].length && winning_hash[:word].length != 10
winning_hash[:word] = word
end
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This complex logic is really readable-- good work on this wave!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants