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

Laura_solar_system #35

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

Conversation

lauracodecreations
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 defines the instance variable that we are going to use throughout the program.
Why do you imagine we made our instance variables readable but not writable? to allow the user to read the value in these variables, but not overwrite them. It gives us control over how we want the program to run because the user cannot write to them.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? I will not be able to apply methods to the planet like planet.name, so the solar system class methodology will have to change to expect and iterate over a hash to find the information that it needs like list planet or find planet by name
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? it will not allow duplicate planets because each key has to be unique
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, my planet class has the responsibility of creating an instance of a planet and a summary about this planet. My solar system has the responsibility of returning information about the planets on its system and adding planets to its system. Planet class has responsibilities of creating and storing information about planets, and the solar system has the responsibility of returning information about its system.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? the main.rb file needs require_relative commands to access the classes for the planet and solar system in order to use the methods from these classes and pass information from one class to another. The planet.rb and solar_system.rb did not need require_relative because they don't use the methods outside its own class.

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) Good
Variable names Good
Git hygiene More commit messages and make your commit messages about the functionality involved NOT the wave completed.
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, nice use of map
find_planet_by_name returns the correct instance of Planet Nice use of select
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 You should really break up that method!
Overall Nice work, you hit all the learning goals. Look at my inline comments and let me know if you have questions. Great job!

require_relative 'solar_system'
require 'pry'

def main

Choose a reason for hiding this comment

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

This method begs to be broken up into smaller simpler methods.

@distance_from_sun_km = distance_from_sun_km
@fun_fact = fun_fact
end
attr_reader :name, :color, :mass_kg, :distance_from_sun_km, :fun_fact

Choose a reason for hiding this comment

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

This should be above initialize and below class Planet

@planets = []
end

attr_reader :star_name, :planets

Choose a reason for hiding this comment

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

ditto with above

end
# map adds a nill if it does not exist
def find_planet_by_name(target_planet)
planets.select { |planet| return planet if planet.name.upcase == target_planet.upcase }

Choose a reason for hiding this comment

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

sweet 👍


def list_planets
string = "Planets orbiting #{@star_name}: \n"
list = planets.map.with_index do |planet, index|

Choose a reason for hiding this comment

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

Awesome 🥇

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