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

Alena's superb Solar_system #43

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

Conversation

Spatterjaaay
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? When we created a new instance of that class methods initialized was immediately initialized, specifically we had all the variables we specified in it ready to use in other methods.
Describe an instance variable you used and what you used it for. I used various instance variables to contain specific attributes of the planets that I used in other methods of that class. For example I used instance variable @distance from the sun in the method distance_from to calculate how far are planets from each other.
Describe what the difference would be if your SolarSystem used an Array vs a Hash. My SolarSystem used an array of instances planets, which meant that I could access them by the index number. If I have used a hash I could have accessed each planet by the key, most likely its name.
Do you feel like you used consistent indentation throughout your code? Yes

@kariabancroft
Copy link

Solar System

What We're Looking For

Feature Feedback
Created Custom Class with initialize method & instance variables. Yep! The Planet and SolarSystem classes.
Used an Array to store a list of planets in the SolarSystem class. Yep!
Readable code with consistent indentation. Yes. Formatting nitpick: You should have one empty line separating your methods. Right now, in the Planet class the only separation is a comment. It's more readable to add an additional empty line.
Created a pull request with your name & a meaningful message. Yes - nice work with the comprehension questions
Nice job doing the distance optional
Your add_planets method in the SolarSystem is using the same variable named planet for two different purposes. This is confusing so this should be re-written to use two different variables, one for the array and one for each individual planet.

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