Skip to content

Branches - Caroline & Paige#5

Open
stupendousC wants to merge 8 commits into
Ada-C12:masterfrom
stupendousC:master
Open

Branches - Caroline & Paige#5
stupendousC wants to merge 8 commits into
Ada-C12:masterfrom
stupendousC:master

Conversation

@stupendousC

Copy link
Copy Markdown

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Signature line (inc. keyword def, method title/name, parameters), block of code, maybe a return statement, end keyword.

What are the advantages of using git when collaboratively working on one code base? | Multiple people can update & see the code. If you mess up, you can go back to a save point. If you want to branch out and try something new in the past, you can.

What kind of relationship did you and your pair have with the unit tests? | Great! We can see what went wrong and figure out how to resolve the bugs.

Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? | .map, .each, .times,

What was one method you and your pair used to debug code? | We used pry to fix a bug where the variable wasn't reassigning to the correct value.

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? | 1. We each see something that the other doesn't see, and that's very cool because it leads to new insights that we might not have had if we worked alone. 2. Being upfront about our own habits and interaction style before actual coding, helps us avoid communication problems and make the work come along much more smoothly than if we had to spend time hammering things out between ourselves.

@stupendousC

Copy link
Copy Markdown
Author

Oops. Incomplete answer on #4. We used enumerables in many places, it was very helpful for iterating through the data and methods like .map was helpful in reducing the line count.

@tildeee

tildeee commented Aug 23, 2019

Copy link
Copy Markdown

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions x, the table formatting broke, which is not a big deal, but just something to note :)
Small commits with meaningful commit messages x, I would probably prefer messages that are less "Wave 2 complete" 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

Well done on this project, Caroline and Paige! Your code overall is extremely easy to understand because your submission has such clean, logical, readable code style and decisions. Great solutions!

I've added a few comments about small refactorings I could imagine happening if you had more time on this project.

That being said, well done overall

Comment thread lib/adagrams.rb
end
end

def draw_letters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of the preliminary set up before the draw_letters method, would this work or could we change it so it would work?

Comment thread lib/adagrams.rb
garbage_index = clone_letters_in_hand.rindex(input_array[index])
clone_letters_in_hand.delete_at(garbage_index)
else
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice!

Comment thread lib/adagrams.rb
return false
elsif input.length == 0
return false
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice! Catch some edge cases before doing the hard work.

Comment thread lib/adagrams.rb
D: 2, G: 2, B: 3, C: 3, M: 3, P: 3, F: 4, H: 4, V: 4, W: 4, Y: 4,
K: 5, J: 8, X: 8, Q: 10, Z: 10 }

word_array = word.upcase.split(//)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Warning: Your indentation here is off, so it makes the rest of the file off in indentation!

Comment thread lib/adagrams.rb
end

####################### WAVE 4 ##########################
words = []

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You don't need this variable, and it's "floating" outside of any method, so feel free to delete this line

Comment thread lib/adagrams.rb
end

return {word: winner, score: highest_score}
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An elegant, clean, readable, straightforward, delicious solution for this method overall!

Comment thread lib/adagrams.rb
all_words = CSV.read('assets/dictionary-english.csv')
input = [input.downcase]
return all_words.include? input
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice work on this optional wave! :)

Comment thread lib/adagrams.rb
all_words = CSV.read('assets/dictionary-english.csv')
input = [input.downcase]
return all_words.include? input
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tests? ;)

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