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 - Dominique #39

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

Leaves - Dominique #39

wants to merge 3 commits into from

Conversation

dtaylor73
Copy link

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 holds instance variables of the class and take in the arguments of the method as values. It runs when an argument is passed to it and is called whenever you call .new on the class.
Why do you imagine we made our instance variables readable but not writable? We made them readable so that ruby can recognize the variable throughout the program, but not writable because we did not want to assign new values to them without creating a new instance of the class.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? Planets would not have any association with the class solar system.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? The planet hash would have to be pre-defined since you cannot push key-value pairs into a hash.
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? Yes - the solarsystem class is responsible for the planet class. The solarsystem class is the parent class of the planet class. You can access the planet class through the solarsystem class but not the opposite.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? I put my require statements at the top of my main.rb and main-wave.rb files. They needed the require statements because they were pulling in the classes solar system and planet from those files. The pattern is that the driver code needs the "require" statements whereas the class files do not.

@dHelmgren
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) good
Variable names good
Git hygiene I'd like to see far more regular commits on a project of this size!
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 yes, but see comments!
Overall Solid job overall! I can see that the learning goals for this assignment were met! Check my comments and keep up the hard work!

require_relative "planet"
require_relative "solar_system"

def main

Choose a reason for hiding this comment

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

I was expecting you to overwrite this file with Wave 3. It did me a scare when there was no loop! Remember, git will track changes, so as long as you commit, nothing is truly lost!

solar_system.add_planet(neptune)

def planet_details(solar_system)
puts "Which planet would you like to learn about?"

Choose a reason for hiding this comment

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

Don't define methods inside other methods unless you are doing something very fancy! This can cause weird problems, and can make debugging a nightmare!

planet_details and add_planet are both defined inside of main!

@planets.each_with_index do |planet, index|
planet_array.push("#{index + 1}. #{planet.name}")
end
return "#{star_name} \n#{planet_array.join("\n")}"

Choose a reason for hiding this comment

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

nice!

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