Skip to content

Leanna/Shelan's Adagram#14

Open
sheland wants to merge 20 commits into
Ada-C10:masterfrom
sheland:master
Open

Leanna/Shelan's Adagram#14
sheland wants to merge 20 commits into
Ada-C10:masterfrom
sheland:master

Conversation

@sheland
Copy link
Copy Markdown

@sheland sheland commented Aug 17, 2018

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method?
What are the advantages of using git when collaboratively working on one code base?
What kind of relationship did you and your pair have with the unit tests?
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful?
What was one method you and your pair used to debug code?
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?

@sheland
Copy link
Copy Markdown
Author

sheland commented Aug 17, 2018

Our answers are in the github folder inside adagrams.rb

@CheezItMan
Copy link
Copy Markdown

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions It looks like you put them in your repo's .PULL_REQUEST_TEMPLATE
Both teammates contributed to the codebase I assume so for this project
Small commits with meaningful commit messages for the most part, however for your commit messages, use terms that describe the functionality added instead of waves completed.
Code Requirements
draw_letters method Check
Uses appropriate data structure to store the letter distribution Very nice work here
All tests for draw_letters pass Well done
uses_available_letters? method Check
All tests for uses_available_letters? pass Well done
score_word method Check
Uses appropriate data structure to store the letter scores Check
All tests for score_word pass Well done
highest_score_from method Check, but see my notes, you have a bunch of redundant code here.
Appropriately handles edge cases for tie-breaking logic
All tests for highest_score_from pass Well done
Overall Overall nicely done. You have a bunch of redundant code in highest_score_from, but overall not bad. See my inline notes and ask me any questions you have. Nice work.

Comment thread lib/adagrams.rb
# #-------------WAVE-1--PASS-----------------
def draw_letters
#string of letters of the correct quantity
letters = ("A" * 9) + ("B" * 2) + ("C" * 2) + ("D" * 4) + ("E" * 12) + ("F" * 2) + ("G" * 3) + ("H" * 2) + ("D" * 4) + ("E" * 12) + ("F" * 2) + ("G" * 3) + ("H" * 2) + ("I" * 9) + ("J" * 1) + ("K" * 1) + ("L" * 4) + ("M" * 2) + ("N" * 6) + ("O" * 8) + ("P" * 2) + ("Q" * 1) + ("R" * 6) + ("S" * 4) + ("T" * 6) + ("U" * 4) + ("V" * 2) + ("W" * 2) + ("Y" * 2) + ("Z" * 1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very good idea

Comment thread lib/adagrams.rb
if letters_in_hand.include?(let) #if the input letter matches a letter in hand
tru_fals << letters_in_hand.delete_at(letters_in_hand.index(let)) #delete letter from the one in hand & push to tru_fals
else
tru_fals << false #if no match, pushes 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.

why not just return false at this point?

Comment thread lib/adagrams.rb
if score == winner[:score] #only exceptions are ties

if winner[:word].length == 10
winner[:word] = winner[:word]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you're swapping the word with itself. Seems unnecessary

Comment thread lib/adagrams.rb
winner[:score] = score

elsif word.length == 10 && winner[:word].length == 10
winner[:word] = winner[:word]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Again repetition, this elsif is redundant.

Comment thread lib/adagrams.rb
winner[:word] = winner[:word]
winner[:score] = winner[:score]

elsif word.length < winner[:word].length && word.length != 10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think this is redundant.

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.

3 participants