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

Solar_System #51

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

Solar_System #51

wants to merge 7 commits into from

Conversation

jazziesf
Copy link

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? Creates an instance of the objects.
Why do you imagine we made our instance variables readable but not writable?
We would make them writable if we wanted the user to modify the information. In this case we do not.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? You would have to interate through the hash each time to look for the information rather than call the method. Your code would be very repetitive.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? The information would be at random rather than an ordered list
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 would say all but one follows the SRP. I should have created a class for user input.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? Only our main program required data from the Solar system and Planets. Planet nor Solar System needed data from each other therefore they did not require file.

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) Good
Variable names Check
Git hygiene Decent number of commits, and good commit messages overall.
Planet
initialize method stores parameters as instance variables with appropriate reader methods Check
summary method returns a string Check
SolarSystem
initialize creates an empty list of planets Check
add_planet takes an instance of Planet and adds it to the list Check
list_planets returns a string Check
find_planet_by_name returns the correct instance of Planet Check
CLI
Can list planets and quit Check
Can show planet details Check
Can add a planet Check
Complex functionality is broken out into separate methods Nope
Overall Nice job, you hit all the learning goals for this project. Great work! I did leave some inline notes in your code. Let me know if you have any questions.

require "pry"

#storing the planets and adding them to solar_system WomenofCode
def main

Choose a reason for hiding this comment

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

This method is just begging to be broken up into smaller methods 🙏

return planet
end
end
puts "That planet is not in our Solar System. Please re-enter your reponse"

Choose a reason for hiding this comment

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

This method should not use puts. Instead just return nil


def find_planet_by_name(found_planet)

@planets.each do |planet|

Choose a reason for hiding this comment

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

You could also do this with:

return @planets.find do |planet|
  planet.name.downcase == found_planet.downcase
end

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