Create Pipes - Angela - solar_system.rb #32

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.

Sign up for GitHub

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

Jump to bottom

Open

awilson2017 wants to merge 2 commits into Ada-C8:master from awilson2017:master

awilson2017 commented Aug 16, 2017

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question	Answer
What was the purpose of the `initialize` method in your class?	to construct the object
Describe an instance variable you used and what you used it for.	@name was one of my instance variables. It was used within my object class to do stuff.
Describe what the difference would be if your `SolarSystem` used an `Array` vs a `Hash`.	With an array we could store limited information like only name. Hashes can store many details about each planet.
Do you feel like you used consistent indentation throughout your code?	yes.


          Create Pipes - Angela - solar_system.rb

03957c5

droberts-sea commented Aug 17, 2017

Solar System

What We're Looking For

Feature	Feedback
Created Custom Class with initialize method & instance variables.	Yes
Used an Array to store a list of planets in the SolarSystem class.	Yes
Readable code with consistent indentation.	Yes
Created a pull request with your name & the template questions answered.	Mostly - "It was used within my object class to do stuff" doesn't quite meet the bar.

This is a good start. While it's true that you didn't get all the UI work for wave 3, it seems like you hit the learning goals around working with objects, which are the most important goal for this project.

I've included some specific comments inline below.

droberts-sea reviewed

View reviewed changes

Pipes - Angela - solar_system.rb

+                def name_of_planet
+                  return @name
+                end

droberts-sea Aug 17, 2017

Since you've got attr_reader :name up top, you don't really need this method. Instead of saying planet.name_of_planet, a caller could just use planet.name.

Pipes - Angela - solar_system.rb

+                def planets_name
+                  list_planet_names = "#{@name}\n"
+                  return list_planet_names

droberts-sea Aug 17, 2017

I appreciate the dedication to making things into methods, but this is probably overkill. In general, you should leave output formatting (like putting newlines at the end of strings) to parts of your code dedicated to interacting with the user.

Pipes - Angela - solar_system.rb

+                  solar_system_output = ""
+                  @solar_system.each do |planet_object| # planet_object represents a new planet instance
+                    #add to planet_output string
+                    solar_system_output += planet_object.planet_output

droberts-sea Aug 17, 2017

Here, I like that you call the existing planet_output method on the Planet class. Makes this method concise and readable.

Pipes - Angela - solar_system.rb

+                def planet_detail
+                  planet_details = "
+                  #{@name}'s volume = #{@volume}.

droberts-sea Aug 17, 2017

This method looks very similar to planet_output above? Could you combine the two somehow?

Pipes - Angela - solar_system.rb

+              planets_wave_2 = [
+                Planet.new("Angela", 15, 20, "no", 1),
+                Planet.new("Tamira", 10, 2, "yes", 2),
+              ]

droberts-sea Aug 17, 2017

I like that you put your collection of planets directly into an array here rather than assigning each to its own variable. That makes it clear to the reader that the planets will only be used through the collection, not individually, and saves you having to come up with variable names.


          Create Pipes - Angela - Solar_system_UPDATE.rb

11a1654

droberts-sea commented Aug 22, 2017

This looks good!

I'm glad you had the time to go back and make these changes - it's a great learning opportunity, but so often in a course like this as soon as one assignment is finished we're immediately off to the next thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet