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

Sockets - Nara #27

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

Sockets - Nara #27

wants to merge 8 commits into from

Conversation

bonara
Copy link

@bonara bonara 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? Initialize method runs when a new instance of a class is created. It takes arguments and sets them for the new instance of the class.
Why do you imagine we made our instance variables readable but not writable? We made our instance variables readable only because we don't want users to change any variables in our already created instances of classes.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? If each planet was stored as a Hash, my summary method would have been more verbose as we need the right key to access the values for the planet details.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? If the SolarSystem class used a Hash, I would still use the each method for my loop; however, getting the name of the planet will be bit complicated.
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? I believe my classes follow SRP. My Planet class is responsible for creating a blueprint for planet instances, and has a method that can be called on a planet instance. Same goes for the SolarSystem class.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? I used 2 gems; thus, added them to the planet.rb and solar_system.rb files. Since the main.rb file is using classes and methods from both planet.rb and solar_system.rb files, I added require_relative planet.rb and solar_system.rb to the main.rb file.

elsif user_input == "add planet"
name = input_handle('name')
color = input_handle('color')
mass_kg = validate_integer('mass_kg')

Choose a reason for hiding this comment

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

The contents of this elsif would be a good candidate for a helper method!

@dHelmgren
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) yes
Variable names yes
Git hygiene I would like to see more descriptive commit messages. Your commits should tell the reader a story about what changed when and why, without them having to dig through the code or project requirements.
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 yes
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 Could be a little better. I like your validations, but see my comment!
Overall Good job overall - this submission is well-organized and clearly hits the learning goals. Keep up the hard work!

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