Pipes - Bennett - Solar System #31
Conversation
bennettrahn
changed the title
Solar SystemWhat We're Looking For
Great work overall! I've commented on a bunch of little things below, but in general I'm quite happy with the code you've submitted. |
| # method to access the planet class by name so as to use planet methods on it | ||
| def planets_by_name(name) | ||
| planet_names = [] | ||
| @planets.each do |planet_class| |
There was a problem hiding this comment.
I really like the idea of making this its own method. Very concise bit of functionality.
You're implementation is a little odd to me though - do you really need to pull out the planet names? Why not:
@planets.each do |planet|
if planet.name == name
return planet
end
return nil
end| all_planets(:name).length.times do |i| | ||
| planet_list += "#{i + 1}. #{all_planets(:name)[i]}" | ||
| if type == :line | ||
| planet_list += "\n" |
There was a problem hiding this comment.
This is a clever way to make your functionality more flexible. Here is a question: Why not pass in the whitespace you want as type, instead of a symbol representing it?
| # method to create a table with info from each planet called by the array passed to table_create | ||
| def table_create(table_arr) | ||
|
|
||
| planet_table_headings = ["Name", "|Mass", "|Index from Sun", "|Circumference", "|Color", "|Year Length", "|Distance from the Sun"] |
|
|
||
|
|
||
| mercury_class = Planet.new("Mercury", 3.285e+23, 1, 9525, "Orange", 0.24, 0.39) | ||
| venus_class = Planet.new("Venus", 4.87e+24, 2, 23628, "Yellow", 0.62, 0.72) |
There was a problem hiding this comment.
You use the term class often in variable names. I would recommend avoiding this - in general, it's usually clear when you've got an instance of a class. Instead, calling these mercury or venus might be more readable.
| mars_class = Planet.new("Mars", 6.39e+23, 4, 13263, "Red", 1.88, 1.52) | ||
| jupiter_class = Planet.new("Jupiter", 1.898e+27, 5, 272946, "Striped", 11.78, 5.2) | ||
|
|
||
| our_solar_system = SolarSystem.new("Milky Way", 1.32e+10, [mercury_class, venus_class, earth_class, mars_class, jupiter_class]) |
There was a problem hiding this comment.
Since you're creating all these local variables just to add them to an array, a more concise way to do this might be:
planet_array = [
Planet.new("Mercury", 3.285e+23, 1, 9525, "Orange", 0.24, 0.39),
Planet.new("Venus", 4.87e+24, 2, 23628, "Yellow", 0.62, 0.72),
# ...
]
our_solar_system = SolarSystem.new("Milky Way", 1.32e+10, planet_array)This makes it clear to the reader that we won't be using the planets individually, only through the collection, and also saves you having to come up with a bunch of variable names.
2 participants
Solar System
Congratulations! You're submitting your assignment.
Comprehension Questions
initializemethod in your class?SolarSystemused anArrayvs aHash.