Skip to content

Leaves - Julia K#27

Open
Kalakalot wants to merge 6 commits into
Ada-C12:masterfrom
Kalakalot:master
Open

Leaves - Julia K#27
Kalakalot wants to merge 6 commits into
Ada-C12:masterfrom
Kalakalot:master

Conversation

@Kalakalot

Copy link
Copy Markdown

Solar System

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
When does the initialize method run? What does it do? The initialize method/constructor runs when an instance of the class is instantiated. When provided with the correct arguments, it creates a new instance of the class.
Why do you imagine we made our instance variables readable but not writable? Because we don't want a user to overwrite the instance variables.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? It would be MUCH harder to access the information associated with the planet because instead of using human-ish language like jupiter.name we'd have to use the not-at-all-human syntax for accessing elements within a hash.
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? I think I would have to create more variables to access the information because that's the only way I would be able to keep track of what I'm trying to do. Otherwise the syntax would be wildly unreadable like it was for the Rideshare project.
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? Ha! No. The classes that I created per the instructions mostly follow SRP, but the main method has a whole bunch of tasks shoved in there: it instantiates the existing planets and adds them to my solar system, it creates an command line interface for the user to (among other things) add a new planet, and then it adds the newly-created planet to the solar system.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? If a file called a method defined in another file, then I had to "require" the file the method was defined in.

@dHelmgren

Copy link
Copy Markdown

Solar System

What We're Looking For

Feature Feedback
Baseline
Questions I would argue that your CLI does have a single responsibility. It's job is to create an environment that can talk to the user! There are a lot of pieces that feed into that, but it is one job.
Whitespace (indentation, vertical space, etc) yes
Variable names yes
Git hygiene This is a pretty big project, I'd like to see more regular commits.
Planet
initialize method stores parameters as instance variables with appropriate reader methods yes
summary method returns a string yes
SolarSystem
initialize creates an empty list of planets yes
add_planet takes an instance of Planet and adds it to the list yes
list_planets returns a string yes
find_planet_by_name returns the correct instance of Planet yes
CLI
Can list planets and quit sort of. The program automatically exits after one prompt is complete.
Can show planet details yes
Can add a planet yes
Complex functionality is broken out into separate methods yes
Overall Good work overall! Your code properly implements all of Planet and Solar_System. However, in the text for section 3, we asked for you to create a control loop that repeatedly prompted the user, but the CLI you submitted didn't meet that criteria. Keep an eye out for vocab like this. If you are ever unsure what's being asked, please check in!

Comment thread planet_test.rb
Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new

describe 'Planet' do
it 'can output a summary' do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A good start, but you should have a handful of tests, ideally at least one or two testing each method.

Comment thread main.rb

main

case command

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This case statement ought to be wrapped in a loop that waits for the command to exit. Additionally, our suggestion was that it would go inside of the main method.

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