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

Leaves -- Janice #41

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

Leaves -- Janice #41

wants to merge 14 commits into from

Conversation

jaitch
Copy link

@jaitch jaitch commented Aug 21, 2019

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? The initialize method runs when I create a new instance of the Planet or Solar System class. It calls the new method and hands it a bunch of information.
Why do you imagine we made our instance variables readable but not writable? Because the user doesn't need to (and shouldn't) change any of those instance variables.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? Manipulating the data stored in each Hash would be cumbersome and confusing.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? It would be a strange choice because most of the data is specific to each planet.
There is a software design principle called the SRP. The Single Responsibility Principle (SRP) says that each class should be responsible for exactly one thing. Do your classes follow SRP? What responsibilities do they have? Hmm. I think some of the methods, or parts of methods, with input/output within the Solar System class should belong in main rather than in a class. I realized this too late. I think the planet- and solar system-related responsibilities are fairly well assigned.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? main has the require statements, because that is the file pulling in information from the other others. I could have had a hierarchy, where main pulled from solar_system and solar_system pulled from planet.

@dHelmgren
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) yes
Variable names yes
Git hygiene great!
Planet
initialize method stores parameters as instance variables with appropriate reader methods yes
summary method returns a string yes
SolarSystem
initialize creates an empty list of planets yes
add_planet takes an instance of Planet and adds it to the list yes
list_planets returns a string yes
find_planet_by_name returns the correct instance of Planet
CLI
Can list planets and quit yes
Can show planet details yes
Can add a planet yes
Complex functionality is broken out into separate methods yes, but see comment
Overall Good work overall. I can tell that the learning goals for this assignment were met. Take a look at my comment and keep up the hard work!

found_planet = find_planet_by_name(planet_sought)
while found_planet.class == Array
puts "Sorry, I don't recognize that planet. Please try again."
print "Which planet are you curious about? "

Choose a reason for hiding this comment

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

let your cli handle this puts-type stuff, to keep this more of a single responsibility.

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