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

Train hash and passenger struct #3

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

Conversation

lorio
Copy link

@lorio lorio commented Aug 26, 2013

No description provided.

end

def city
city = "New York"
Copy link
Member

Choose a reason for hiding this comment

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

In ruby, you don't need to have "city = " here. if you wanted to have method that returns "New York", you should:

def city
  "New York"
end

More likely, you really wanted to do this (outside of a method)

city = "New York"

Copy link
Author

Choose a reason for hiding this comment

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

Thanks...fixed them .. but on trying a pull request:
Pull request creation failed. Validation failed: A pull request already exists for lorio:master.
but they do work fine. Will move on.

On Aug 26, 2013, at 5:00 PM, Jesse Wolgamott [email protected] wrote:

In passenger2.rb:

@@ -0,0 +1,15 @@
+def name

  • name = "Lori"
    +end

+def city

  • city = "New York"
    In ruby, you don't need to have "city = " here. if you wanted to have method that returns "New York", you should:

def city
"New York"
end
More likely, you really wanted to do this (outside of a method)

city = "New York"

Reply to this email directly or view it on GitHub.

Copy link
Member

Choose a reason for hiding this comment

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

GitHub pulls automatically update when you push to the branch you originally created on. So I see the changes you made.

@jwo
Copy link
Member

jwo commented Aug 26, 2013

Hi! Good job here, it got the job done -- though I think some elements were a bit confusing. Let me know if you have questions:

I couldn't follow the reasoning here:

train_parts = {}
train_parts[:train] = train

puts "Train:"
train_parts[:train].each #

Specifically, I'm not sure the train_parts ever really gets used. If you wanted to see all of the key/values that your train hash had, you should just keep it as:

train = {}
train[:city] = "New York"
train[:engines] = 1
train[:cars] = 6

puts "Train:"
train.each do |key, value|
  puts " * #{key}: #{value}"
end

puts " * and a caboose."




Passenger = Struct.new(:city, :name) do
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you want to have the "do" here.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry. Passenger 3 and train.rb are my new solutions. What is the best way to delete...is it best to delete the earlier tries right from my git repo? Or from my local project directory?

On Aug 27, 2013, at 5:45 PM, Jesse Wolgamott [email protected] wrote:

In passenger.rb:

@@ -0,0 +1,16 @@
+
+
+
+
+Passenger = Struct.new(:city, :name) do
I don't think you want to have the "do" here.


Reply to this email directly or view it on GitHub.

Copy link
Author

Choose a reason for hiding this comment

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

In future I'll just push my best effort instead of starting new ones.

Lori
http://artanddesign.us

On Aug 27, 2013, at 5:45 PM, Jesse Wolgamott [email protected]
wrote:

In passenger.rb:

@@ -0,0 +1,16 @@
+
+
+
+
+Passenger = Struct.new(:city, :name) do

I don't think you want to have the "do" here.


Reply to this email directly or view it on
GitHubhttps://github.com//pull/3/files#r6019436
.

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