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

Ana Lisa Sutherland- Solar System- Octos C9 #45

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

Conversation

The-Beez-Kneez
Copy link

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
What was the purpose of the initialize method in your class? initialize prompted the user to enter the information for the instance variables I assigned in my class method. Every time a new instance of my class was created these instance variables were required.
Describe an instance variable you used and what you used it for. I used @name and @color in my Planet class to ensure that every new planet that was created had a user entered name and color stored.
Describe what the difference would be if your SolarSystem used an Array vs a Hash.
Do you feel like you used consistent formatting throughout your code? I attempted to...

@CheezItMan
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Readable code with consistent indentation. Good indentation! See my in-line notes for readability and naming.
Primary Requirements
Created Custom Solar System Class with initialize, add planet & list planets methods, without using puts. Check
Planet Class Created Check
Created a collection of Planet objects as an instance variable in SolarSystem. Check
Accessor methods created Check
Method created to return the Planet's attributes and not use puts Check
Created a user interface to interact with the SolarSystem including adding a planet and viewing a planet's details It work, but it's a bit awkward. See my in-line notes.
Additional Notes You didn't answer: Describe what the difference would be if your SolarSystem used an Array vs a Hash. You also submitted 2 files. I graded the one I thought was the finished product. Let me know if I was wrong. You hit all the primary requirements. I made some comments in your code for minor issues. Please review them. Let me know if you have questions.

@@ -0,0 +1,94 @@
class Solar_S

Choose a reason for hiding this comment

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

In general don't use underscore (_) in Class names. Instead naming this class SolarSystem would be more appropriate.

# adds the new planet created by user to the whole solar system array
def planet_bio
planet_sum = []
planet_sum << "Name:#{name}\nColor:#{color}\nOrder In Solar System:#{order}\nYear Length:#{year_length}\nDistance From The Sun:#{@distance_from_the_sun}"

Choose a reason for hiding this comment

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

Why is planet_sum an array? Why not make it just a String? I don't think you gain anything making it an array.

Planet.new("Pluto","Grey","9th","90,500 Earth days","7.38 BM")
]
my_universe = Solar_S.new(milky_way)

Choose a reason for hiding this comment

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

I'd put this in some kind of loop so I could see or add multiple planets.


if welcome_choice == "view"
puts "Good choice, let me show you our planets"
puts my_universe.summary

Choose a reason for hiding this comment

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

If the Planet name includes a number, it would be good to make sure that number works. Otherwise it's counterintuitive.


class Planet
## Create reader methods to give a user access to read the instance variables.
attr_accessor :name, :color, :order, :year_length, :distance_from_the_sun

Choose a reason for hiding this comment

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

Why make these attr_accessor unless you specifically want to give users the ability to directly change the attributes using attr_reader would be more appropriate.

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