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

Sockets-Chantal #47

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

Conversation

ChantalDemissie
Copy link

@ChantalDemissie ChantalDemissie commented Feb 27, 2019

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 runs when we make a new instance (ie planet.new) its responsibility is to assign values to variables.
Why do you imagine we made our instance variables readable but not writable? so the user cant alter the original instance variables. //re-answer after wave 3 maybe?
How would your program be different if each planet was stored as a Hash instead of an instance of a class? i think my program would have more lines of code to interpret we would not be able to have any complex behaviors like summary
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? We would need to write alot more code to access any of the instances
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 they do. the planet class tracks the planet data. the solar system tracks multiple instances of planets
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? i put them at the top of my file. Main.rb is the only one that needed require statements. solar_system.rb and planet.rb did not. main.rb is the only one that needed to acess the information/variables from the other two files. The pattern is if it needs to pull information from somewhere it must use require statements to acess it.

still working on wave 3
@ChantalDemissie ChantalDemissie changed the title Add files via upload chantal-sockets.rb Feb 27, 2019
@ChantalDemissie ChantalDemissie changed the title chantal-sockets.rb Sockets-Chantal Feb 27, 2019
wave 3 list planets, planet details working.
@droberts-sea
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) yes
Variable names yes
Git hygiene At this point you should be using the command line to add, commit and push files. If this is still unclear, come and find an instructor - this is a key skill.
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
Overall

Good job overall. Your code is clean and well-organized, and you definitely hit the project requirements. However, I am concerned that you may have missed learning goals around working with git, and around using helper methods with classes. Please review these two topics, and feel free to reach out if you have questions.

def add_new_planet
puts "What is the name of the planet?"
name = gets.chomp.capitalize
puts "What color is the planet?"

Choose a reason for hiding this comment

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

I like that you broke this out into a separate method - good organization!

elsif input == "add planet"
planet_new = add_new_planet
solar_system.add_planet(add_new_planet)
else

Choose a reason for hiding this comment

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

On line 30 you call add_new_planet and assign it to the planet_new variable. Then on line 31, instead of using planet_new, you call add_new_planet again! That means the user has to enter all this info twice.

Copy link
Author

Choose a reason for hiding this comment

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

THANK YOU for pointing that out. that was driving me bananas i couldnt figure out why it asked twice.


def name
return @name
end

Choose a reason for hiding this comment

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

You should use the helper methods (attr_reader, attr_writer, attr_accessor) to generate these getter and setter methods.

def find_planet_by_name(planet_name)
@planets.each do |planet|
if planet_name.upcase == planet.name.upcase
return planet

Choose a reason for hiding this comment

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

Is there an enumerable method you could use for this?

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