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

Amber Lynn/Barbara W., Edges, Adagrams.rb #10

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

Conversation

griffifam
Copy link

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? parameters, local variables, method signature, method name, and a return value
What are the advantages of using git when collaboratively working on one code base? Able to share updates to codes rather seamlessly and stay up to date on changes
What kind of relationship did you and your pair have with the unit tests? Testing went well up until the tie-breaking criteria portion. It was difficult because we believed we need to ALSO have a working program, to not only pass the tests but have working code.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? No, that might have been helpful.
What was one method you and your pair used to debug code? We used puts; although time-consuming; it was effective and we used a conditional statement to populate a testing_array to check if user input was valid.
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? I, Amber Lynn, need to rubber duck more frequent to work through the logic in problems. Also, the visuals are VERY helpful. Soooo...(we talked with Dan) .either due to our misunderstand or lack of asking for assistance we believed we had to pass the 16 tests AND have a working program that was written from scratch. Both were achieved...just not at the same time. In the future, we need to get clarification on the required output for each assignment. So regarding the program we wrote...adagrams/lib/adagrams.rb passes 16/16 tests but does not run on the terminal correctly UNLESS line 61 is uncommented. While adagrams/adagrams.rb runs correctly and scores correctly on the terminal but only passes 15/16 tests.

@tildeee
Copy link

tildeee commented Aug 24, 2018

Adagrams

What We're Looking For

Feature Feedback
General
Answered comprehension questions x
Both teammates contributed to the codebase x, didn't practice pushing from both accounts though (this is okay, just something to practice next time)
Small commits with meaningful commit messages would probably be better to have messages about what the code changes do, rather than "implements wave X", otherwise good job!
Code Requirements
draw_letters method x
Uses appropriate data structure to store the letter distribution x
All tests for draw_letters pass
uses_available_letters? method x, I think your strategy and angle of thinking is correct, but there are some tricks in programming you can do to make it more concise and more expressive!
All tests for uses_available_letters? pass
score_word method x
Uses appropriate data structure to store the letter scores x
All tests for score_word pass
highest_score_from method x
Appropriately handles edge cases for tie-breaking logic x
All tests for highest_score_from pass
Overall

Hi all!

Actually, to get your code to run the tests, I did the following:

  • I reverted the tests back to the given tests that the original project gave
  • I renamed your method highest_score_from_words and all calls to it to the name highest_score_from, so that the tests could run it

With those two changes, I got all of the tests to pass except for four -- and they weren't very far off.

That being said -- I am so sorry that you all misunderstood the assignment requirements and wrote the game code! Please let us know how to make it more clear, and take care to follow the project requirements. If it isn't clear at this moment: we really just want you all to implement code within methods now. The code in the methods will be run and tested through the tests alone.

I think otherwise the code is good. All of the methods get the answers we're looking for... The only failing tests I see are for highest_score_from method, and are caused because you put an array of words into the final_winner_hash[:word] rather than a single entry. I'm putting a comment here.

There are some areas of code that, when given a set of fresh eyes, I think you will see can be refactored to do the same work, but with a simplified approach. I'll leave some comments for that.

Good work nonetheless

lib/adagrams.rb Outdated
else
if number_of_winning_words_of_min_length == 1

final_winner_hash[:word] = all_winning_words_of_shortest_length
Copy link

Choose a reason for hiding this comment

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

Here, you are assigning into final_winner_hash[:word] the value of the entire array all_winning_words_of_shortest_length... even if this array is one element long, you'll still want to take it out of this array with all_winning_words_of_shortest_length.first or all_winning_words_of_shortest_length[0].

return true
else
return false
end
Copy link

Choose a reason for hiding this comment

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

In this method, you all create testing_array to represent an array of booleans (trues or falses) for each letter if the letter was found... if the letter was not found, you put into testing_array a false value.

In the end, you ask if testing_array.all?, which asks, "if the testing array is all true values, this is true".

We know that using return will stop a method and return a value. Specifically, we know that if a code reaches a line and executes return false, it will stop proceeding into the method and get out of the method and return false.

What if instead of approaching this problem with "Let's get an array of trues and falses, and if there's a false, then the answer "uses_available_letters?" is false"... We approach it with, "if we need to shovel in false into testing_array once, then we know that the answer is false"?

For example, on line 53, you all wrote testing_array << false. Is there any case in which shoveling false into testing_array does not mean that the method should return false overall?

lib/adagrams.rb Outdated
end

testing_array = []
editted_letters_drawn = letters_drawn_input
Copy link

Choose a reason for hiding this comment

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

you are assigning the value of letters_drawn_input into a new local variable editted_letters_drawn. Is there a reason why you couldn't just keep using letters_drawn_input?

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