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_Myriam #44

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

Ports_Myriam #44

wants to merge 4 commits into from

Conversation

MyriamWD
Copy link

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? It runs when .new is used and it "constructs" the object.
Why do you imagine we made our instance variables readable but not writable? Because the user should not modify the state from outside the class.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? Accessing the descriptive values for each planet would have been more difficult.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? I would have had to define a key for each instance of planet and use that key to display the summary of a planet for example.
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 tried to make each class have their own responsibilities but I'm not sure I'm achieving it.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? main.rb is where I'm using require_relatives because it is the file where the classes interact.

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Overall You hit the learning goals of the project, but your find_planet_by_name method is a bit awkward. See my inline notes about it and let me know if you have questions. Nice work.

end

def find_planet_by_name(planet_name, match)
@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 simply:

return planets.find do |planet|
  planet.name.upcase == planet_name.upcase
end

end
end

def distance_between(planet_q)

Choose a reason for hiding this comment

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

This isn't calculating the distance between two planets. Instead it's calculating the distance between a planet and the sun, which is already served by a method in the Planet class.

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.

None yet

2 participants