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

Branches - Caroline #31

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

Conversation

stupendousC
Copy link

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? runs as soon as the class object method .new is called. It makes a new instance object of that class.
Why do you imagine we made our instance variables readable but not writable? It's for cases where you only want the instance variables to be assigned under specific conditions (such as in a writer method), to prevent someone accidentally writing over the actual value with garbage.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? Hashes are objects too, so I'd just access the instance variables slightly differently. Like, I may have to call an instance variable via its hash key instead of @something. But storing it as a hash can make my object vulnerable to accidental overwrites, because it doesn't have the protection that @variables have with attr_reader.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? It'd be fine, I'll just have to access my planet objects slightly differently.
There is a software design principle called the SRP. 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? I haven't read that chapter in POODR yet, but I think so? Planet class only minds its own attributes and reports on them. SolarSystem class does include planet objects in its attributes and even though it can access the attributes of those objects, it does not alter them. So I think the answer is yes...?
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? the planet.rb and solarsystem.rb need to be required, otherwise main.rb would not know what these classes are. i also added a colorize gem. all require statments go on the top, per convention. tests.rb does NOT need to be included in main.rb because main.rb does not depend on tests.rb. Basically, if a file depends on a different file to run, it will need to require that file.

@jmaddox19
Copy link

Solar System

What We're Looking For

Feature Feedback
Baseline
Whitespace (indentation, vertical space, etc) X
Variable names X
Git hygiene You'll probably want to start committing is smaller commits. (ie. When you finish making the planet class, when you make the planet_details method, etc.) It's a hard habit to get into but as the projects get bigger, things will get harder to keep track of and you'll break something and you'll want a recent working version to go back to.
Planet
initialize method stores parameters as instance variables with appropriate reader methods X
summary method returns a string X
SolarSystem
initialize creates an empty list of planets X
add_planet takes an instance of Planet and adds it to the list X
list_planets returns a string X
find_planet_by_name returns the correct instance of Planet X
CLI
Can list planets and quit X
Can show planet details X
Can add a planet X
Complex functionality is broken out into separate methods For the most part!
Overall Great!

LOVE your fun facts! Hahaha 🤣

Your code is SO THOROUGH, validating everything which is so cool! Definitely shows you're thinking like a programmer.

The one thing that could've made main more readable is making a separate method for each user input option. That way the main controller code would just read like "if user_input is 'planet details' call planet_details method", etc.

Great work!

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