Skip to content

Leaves - Cloudy#50

Open
OhCloud wants to merge 2 commits into
Ada-C12:masterfrom
OhCloud:master
Open

Leaves - Cloudy#50
OhCloud wants to merge 2 commits into
Ada-C12:masterfrom
OhCloud:master

Conversation

@OhCloud

@OhCloud OhCloud commented Aug 21, 2019

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 'turns on' the initial values for the objects (with arguments)
Why do you imagine we made our instance variables readable but not writable? bc then you could potentially change(write) all of them and that would be bad bc they would keep changing.
How would your program be different if each planet was stored as a Hash instead of an instance of a class? it wouldnt be able to call it as i have
How would your program be different if your SolarSystem class used a Hash instead of an Array to store the list of planets? when we add in new planets, we would need more info (a key, and a valuessss for all the different types of stuff) and they way to write it and store is different and would be a bigger pain.
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? well yes and no bc my main is outta control. i have alot of things happening there. i do try to use small methods that do only one thing as to avoid tripping over myself, usually.
How did you organize your require statements? Which files needed requires, and which did not? What is the pattern? i did not!

@dHelmgren

Copy link
Copy Markdown

Solar System

What We're Looking For

Feature Feedback
Baseline
Comprehension Questions You do have some organization to your require statements! There is however, a handy tip in my comments.
Whitespace (indentation, vertical space, etc) Some issues, see comment.
Variable names Good
Git hygiene On a project of this size I would expect to see you commiting more regularly!
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 yes
Can show planet details yes
Can add a planet yes
Complex functionality is broken out into separate methods no
Overall This submission has a lot of good parts. Particularly, your Planet and Solar_System classes are very well put together. There are a few comments about how this project could be improved, but keep up the hard work!

Comment thread main.rb
@@ -0,0 +1,74 @@
require_relative 'planet'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fun fact, since Solar_system.rb requires 'planet', this is not necessary!

Comment thread main.rb
puts "Do you want to play again? Yes or No?"
answer = gets.chomp
if answer == "yes"
main_menu

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 method call turns this from a loop into a recursive call! Don't do this unless you have a very good reason.

In other words, avoid the situation where either:

  • method_a calls method_a
  • or
  • method_a calls method_b calls method_a

Comment thread main.rb

user_input = "y"

while user_input == "y"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

user_input never changes, Which is not a great way to write a while loop. You rely on a break keyword at the end of this section to end the loop, rather than changing the value of user_input. In other words, what you've written is equivalent to:

while true
   if [some exit logic]
       break
   end
end

Comment thread solarsystem.rb

def list_planets
list = "\nPlanets orbiting #{@star_name}:"
@planets.each_with_index do |planet, index|

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your code is really squished together vertically. I like to use new lines between separate "thoughts" in my code to make it easier to digest.

Comment thread solarsystem.rb
list += "\n#{index + 1}. #{planet.name}"
end
return list
main

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this here?

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