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

Just Panda #9

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

Just Panda #9

wants to merge 6 commits into from

Conversation

var114
Copy link

@var114 var114 commented Dec 2, 2013

I still haven't updated my spec. Thanks.

def route
CalculatesRoute.calculate(cities)
def find_point(start_name)
start = @cities.select {|city| city if city.name == start_name }
Copy link
Member

Choose a reason for hiding this comment

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

You can simplifiy this select to

start = @cities.select {|city| city.name == start_name }

The reason is that select works by keeping entries in the array where the block returns true. A simply example:

[1,4,5,6].select{|i| true}
=> [1,4,5,6]

Copy link
Member

Choose a reason for hiding this comment

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

Further, since you are getting the first item, you can change to a find

def find_point(start_name)
  @cities.find {|city| city.name == start_name }
end

This works where it early escapes after finding a true element

[2,3,3,3,4,5].find {|i| i.odd?}
=> 3

@jwo
Copy link
Member

jwo commented Dec 2, 2013

Excellent job! Especially nice job on the spec, with both should-equal and message expectations (should_receive).

👍 🎉

@var114
Copy link
Author

var114 commented Dec 4, 2013

Hey Jesse,

My code is a mess still. I can’t get benchmark to work…and my Rspec is all over the place. but I wanted to upload it…instead of just sitting on it and waiting for it to hatch.
On Dec 2, 2013, at 10:08 AM, Jesse Wolgamott [email protected] wrote:

Excellent job! Especially nice job on the spec, with both should-equal and message expectations (should_receive).


Reply to this email directly or view it on GitHub.

@@ -1,7 +0,0 @@
Copyright (c) 2012 Jesse Wolgamott
Copy link
Member

Choose a reason for hiding this comment

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

Please leave the license files. (unless you plan to delete your repo)

@jwo
Copy link
Member

jwo commented Dec 4, 2013

Some promise here! The spec's don't pass though... want to take another run at the specs?

@jwo
Copy link
Member

jwo commented Dec 4, 2013

I’d forget on the benchmarks for now. I think your functionality is great; focus on the specs and you’ll be 👍

On Monday, December 2, 2013 at 11:06 AM, var114 wrote:

I still haven't updated my spec. Thanks.
You can merge this Pull Request by running
git pull https://github.com/var114/Episode7 master
Or view, comment on, or merge it at:
#9
Commit Summary
Panda

File Changes
M lib/calculates_route.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-0) (17)
M lib/map.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-1) (9)
M lib/place.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-2) (12)
M lib/sales_person.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-3) (15)
M salesperson.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-4) (16)
M spec/calculates_route_spec.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-5) (22)
M spec/map_spec.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-6) (25)
M spec/place_spec.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-7) (54)
M spec/sales_person_spec.rb (https://github.com/RubyoffRails/Episode7/pull/9/files#diff-8) (67)

Patch Links:
https://github.com/RubyoffRails/Episode7/pull/9.patch
https://github.com/RubyoffRails/Episode7/pull/9.diff

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