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

Michelle & Aurea - Edges - Adagrams #8

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

Conversation

kangazoom
Copy link

Adagrams

Congratulations! You're submitting your assignment.

Comprehension Questions

Feature Feedback
What are the components that make up a method? Parameters and a code block with a return statement
What are the advantages of using git when collaboratively working on one code base? We can access and update the same code on different computers. The driver can work on their own computer without needing to switch. Also, we can keep track of who made which changes.
What kind of relationship did you and your pair have with the unit tests? It was a good way to check if our code was working and helped us write it. We also wrote some tests for wave 5, which helped us better understand our code and the written requirements.
Does your code use any methods from the Enumerable mixin? If so, where and why was it helpful? Yes, we used .sum for wave 3. It helped us add scores together, but we only needed one line.
What was one method you and your pair used to debug code? We used pry, evaluated exceptions we found in the trace stack, and also printed results.
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 thought we handled a good system with switching driver/navigator roles with after each commit (2) We were happy with how our knowledge of coding and our approaches to problem solving complemented one another since we have different backgrounds and training.

@tildeee
Copy link

tildeee commented Aug 25, 2018

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
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 y'all!

The code looks great, works great, and meets all the requirements. It's very readable! Not a huge comment, but y'all could probably do with less comments -- maybe the comments I appreciated were the more nuanced decisions like the hash for the optional wave 5. I didn't quite need comments for the obvious stuff.

Your implementation of score_word using sum is my favorite implementation of that wave!

I love the use of the hash for the dictionary so accessing it would be more efficient. In the future, if you have to do a weird data structure like this, instead of setting the value to 0, maybe you can choose 1 (which signifies "true" in some languages) or true.

Also, the tests for the optional wave look great.

I don't have any more comments on this code -- great work overall :)

# draw letters

# letter pool
pool = ('A'*9 + 'B'*2 + 'C'*2 + 'D'*4 + 'E'*12 + 'F'*2 + 'G'*3 + 'H'*2 + 'I'*9 + 'J' + 'K' + 'L'*4 + 'M'*2 + 'N'*6 + 'O'*8 + 'P'*2 + 'Q' + 'R'*6 + 'S'*4 + 'T'*6 + 'U'*4 + 'V'*2 + 'W'*2 + 'X' + 'Y'*2 + 'Z').chars
Copy link

Choose a reason for hiding this comment

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

nice 👍


# made a clone of letters; previously, we ran into memory location/duplication issues
# we don't want to alter letters since it needs to reset with all 10 pulled letters available upon each call
letters_copy = letters.clone
Copy link

Choose a reason for hiding this comment

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

We'll talk about reference vs. value in class formally soon. But also, I'm pretty curious, what side effects did you see if you didn't make a copy of letters?

Choose a reason for hiding this comment

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

If you input in an incorrect word, and then on the second try input in a valid word using any of the same letters, the program will not recognize it as valid because it modifies the hand of letters after the first attempt.

Copy link
Author

Choose a reason for hiding this comment

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

If I'm remembering correctly, we had the following issue if we didn't make a copy: if someone drew the letters "a", "b", "c", "d", "e", "f" for example and made the nonsense word "abc" then they would be asked to try again. But "a", "b", and "c" would have been mistakenly deleted from the list, leaving only "d", "e", and "f". So if they next tried the word "cab" they would get an error message. We also tried letters_copy = letters, but I think we figured out that they're pointing to the same place in memory because the "copy" would have the same issue.

Copy link

Choose a reason for hiding this comment

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

Right! Got it, makes sense. Thanks!

@kangazoom
Copy link
Author

Thanks, Dee! Not sure you can see this, but I'm the one who overdid the comments! I'm always afraid I won't know what my code is doing if I look at it months from now. I'd love to have a conversation with you about when comments should/shouldn't be added. Thanks again for your helpful feedback.

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