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 Yasmin Leaves #43

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

Conversation

YasminM11
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 we create an object, it sets some values as part of creating an object(instance of the class)
Why do you imagine we made our instance variables readable but not writable? Because we don't want to change the instance variable 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? we can not build objects from hashes which will make our program longer and not organize
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? array store items only in order but hashes store values and keys
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, my code follow just follow one class
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? on the top of the pages main

@YasminM11 YasminM11 changed the title solar_system Leaves Yasmin solar_system Leaves Aug 21, 2019
@YasminM11 YasminM11 changed the title Yasmin solar_system Leaves solar_system Yasmin Leaves Aug 21, 2019
@beccaelenzil
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) Remove extra white apce
Variable names check
Git hygiene You'll probably want to start committing is smaller commits. (ie. When you finish making the planet class, when you make the planet_details method, etc.) It's a hard habit to get into but as the projects get bigger, things will get harder to keep track of and you'll break something and you'll want a recent working version to go back to.
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 This wave was not required
Can list planets and quit N/A
Can show planet details N/A
Can add a planet Not required
Complex functionality is broken out into separate methods N/A
Overall Good job overall. I am glad you were able to focus on and meet the learning goals of defining classes and using composition.

end
return planet_names
end

Choose a reason for hiding this comment

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

Only have a single space between method definitions

return "#{@name} has a #{@color} color. A fun fact about #{@name}: #{@fun_fact}."
end
end

Choose a reason for hiding this comment

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

This is a small detail, but you should remove extra lines of code.

puts "\n#{earth.summary}"
puts "\n#{mars.summary}"
puts "\n#{neptune.summary}"

Choose a reason for hiding this comment

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

remove this space

def main
earth = Planet.new('Earth', 'blue-green', 5.972e24, 1.496e8, 'Earth is the only planet known to support life')
neptune = Planet.new("Neptune", "blue", 1.02e26, 4.495e8, 'Neptune is the Coldest Planet in the Solar System')
mars = Planet.new("Mars", "red", 6.42e23, 227.9e9, 'Pieces of Mars have been found on Earth')

Choose a reason for hiding this comment

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

You could consider putting this planets in an array, and iterating over that array using each to puts each one.

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