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

Ports - Laneia #43

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

Ports - Laneia #43

wants to merge 11 commits into from

Conversation

laneia
Copy link

@laneia laneia commented Feb 27, 2019

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? Runs when new instance of class is created. It gives default values to instance variables.
Why do you imagine we made our instance variables readable but not writable? When you want to be able to read data but not let the user change it.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? It would be much more complicated iterating through such a long hash (for me anyways).
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? Would have to refactor to call key value instead of value at index.
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? Planet class creates instances of planets, solar system creates solar system and adds planet classes to it. It depends on how broad the definition of single responsibility is - the different methods in solar system do have different functionality so I'm not sure.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? Solar system required planets and main required both of those. The higher level the function of the file is, the more of the lower level files are needed to provide the backbone that's being used.

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Overall Nice work, you hit the learning goals of the project. See my inline notes for minor suggestions. Nice work getting to some testing as well!

end

def add_new_planet
print "Okay, what's the name of the planet we're missing?\n"

Choose a reason for hiding this comment

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

This method should be moved to main.rb That way you would separate the user interface role with the job of keeping and tracking the list of planets.

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