Skip to content

Conversation

@MariaWissler
Copy link

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? assigning an initial value for a data object or variable
Why do you imagine we made our instance variables readable but not writable? so the exercise will be easier to navigate, also for the use of the program as we may not want to display non trustable info maybe ?
How would your program be different if each planet was stored as a Hash instead of an instance of a class? we will have to look for them with key- values
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? handling the data differently, adding corresponding keys, values,
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, planet creates the planets and instances, main runs program and solar system allows main to work providing with all the methods for its functionality
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? my 'require' statements are at the top of main file as it necessary for main to run, didnt see the need to require them in the other files

distance_from_sun = gets.chomp

puts "Planet Fun Fact:"
fun_fact = gets.chomp_to_i

Choose a reason for hiding this comment

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

A couple of weird things are happening here. 1) You use gets.chomp_to_i on line 59 when you mean gets.chomp.to_i as you did on line 53. But, the fun fact isn't a number, it seems you mixed up line 56 and 59. More testing would have probably caught this.

when "list planets"
puts solar_system.list_planets
when "get planet details"
puts solar_system.planet_details.summary

Choose a reason for hiding this comment

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

What if it doesn't find the planet? you should probably return planet.summary from planet_details instead.

return planet
elsif planet.name != name.downcase
puts "no related data found, please enter a listed planet"
planet_details

Choose a reason for hiding this comment

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

This is a very dangerous way to call this method! Calling a method inside itself is called recursion, and if the method has a bug, there is no escape except to shut the program down.

@dHelmgren
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) Yes
Variable names yes
Git hygiene I would like to see more descriptive commit messages. Your commits should tell the reader a story about what changed when and why, without them having to dig through the code or project requirements.
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, but see comment!
CLI
Can list planets and quit yes
Can show planet details yes
Can add a planet no, see comment
Complex functionality is broken out into separate methods Mostly. You have put a lot of the command line work inside of SolarSystem rather than letting it just process data.
Overall This submission is good overall, but it seems like you didn't give yourself enough time to fully test it.

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