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

Brian Forbes #192

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

Conversation

Codeherder19
Copy link

No description provided.

@francepack
Copy link
Contributor

@Codeherder19
Nice work here! Just a few bits of feedback as you continue to prepare for Turing:

  • Really like seeing the progress in your analysis of the checkerboard problem through your pseudocode. Nice work there! How you write pseudocode is an evolving process, and I think it can really help to use some visual elements to keep thoughts organized. I also encourage you to try some more actual coding of these problems in the time leading up to your start date!

  • Nice job using a for loop to print speckled frogs the correct number of times. A couple things I notice here- You'll typically see i++ rather than i-- used in for loops. You can achieve similar loops with both, as the one you have here works just fine, but using i++ is more consistent for other developers understanding, and I find it helps me access elements in an array in a more organized way. Leading to the second thing- I do not see any reference to you numWords array inside of your loop. Rather than just print i in your story, you could access elements of that array by bracket notation using i. You may need to think about how that array is set up versus what element you want to grab when, for example, the first time I run through this loop, the way it is set up now, i starts with a value of 10, so you'd be wanting to grab your first word from numWords based on that value. If you did change to i++, you'd just want to be careful to reconfigure how that is set up.

  • A couple things I wanted to mention that I saw n your class work. First, In your human class, since you have bodyOdor and numberOfDigits listed as parameters within your constructor method, I'd expect to see any instantiation of that class needing to be given that information. Specifically, when you create an instance on line 24, you need to give new Person some parentheses and data to hold in its attributes.
    Second, I like the idea of the interaction between numberOfDigits and woodShopAccident, but I think your method here could be a bit more nuanced. For example, you, instance brian, had a woodShopAccident and now have 9 digits. As it stands, you could call brian.woodShopAccident(10) and brian would have 10 fingers again! I think woodShopAccident seems more like a method where this.numberOfDigits = this.numberOfDigits - input

@Codeherder19
Copy link
Author

Codeherder19 commented Sep 22, 2020 via email

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