Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions LICENSE

This file was deleted.

30 changes: 20 additions & 10 deletions lib/calculates_route.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,32 @@
require_relative "./map"
class CalculatesRoute

def self.calculate(points)
attr_reader :total_distance, :total_time

def self.calculate(points, start_point)
@total_distance = 0
remaining_points = points
route = []
route << remaining_points.slice!(0)
until remaining_points == [] do
next_point = shortest_distance(route.last, remaining_points)
route << remaining_points.slice!(remaining_points.index(next_point))
conf = remaining_points.index(start_point)

if conf.nil?
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 simplify this code using some Ruby coolness.

Given that nil.to_i == 0

And

array = [5, 6, 7]
array.slice! nil.to_i
=>[5]
array
=>[6,7]

We could deal with conf wether it is a number, or zero:

Remove these lines:

conf = remaining_points.index(start_point)
if conf.nil?
  route << remaining_points.slice!(0)
else
  route << remaining_points.slice!(conf)
end

And replace with this one liner

route << remaining_points.slice! remaining_points.index(start_point)

Copy link
Author

Choose a reason for hiding this comment

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

Hey Jesse,

Can you take a look at my new upload. I implemented benchmark…also fixed a good chunk of my specs. I’m still getting 2 failures though.

Also can you explain to me the nil.to_i == 0 thing?

Also I can’t how to I get rid of texas_cities = [], if benchmark needs that array to pull from.

Thanks,
Patil
On Dec 4, 2013, at 12:23 PM, Jesse Wolgamott [email protected] wrote:

In lib/calculates_route.rb:

 remaining_points = points
 route = []
  • route << remaining_points.slice!(0)
  • until remaining_points == [] do
  •  next_point = shortest_distance(route.last, remaining_points)
    
  •  route << remaining_points.slice!(remaining_points.index(next_point))
    
  • conf = remaining_points.index(start_point)
  • if conf.nil?
    You can simplify this code using some Ruby coolness.

Given that nil.to_i == 0

And

array = [5, 6, 7]
array.slice! nil.to_i
=>[5]
array
=>[6,7]
We could deal with conf wether it is a number, or zero:

Remove these lines:

conf = remaining_points.index(start_point)
if conf.nil?
route << remaining_points.slice!(0)
else
route << remaining_points.slice!(conf)
end
And replace with this one liner

route << remaining_points.slice! remaining_points.index(start_point)

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.

Also I can’t how to I get rid of texas_cities = [], if benchmark needs that array to pull from.

You get it below, and can use it later:

texas_cities = texas.css(" .topic-subcategory-link a").map do |node|
  node.content
end

Also can you explain to me the nil.to_i == 0 thing?

Sure -- checkout http://rubyfiddle.com/riddles/d30ea ... you'll see that nil, when cast to an integer, has the value of 0. In the sample code, where you check if conf if nil, and use 0 if it is nil... You could just use conf instead, like this

conf = remaining_points.index(start_point)
if conf.nil?
  route << remaining_points.slice!(conf.to_i)
else
  route << remaining_points.slice!(conf)
end

And, since conf is an index, it's an integer. So you can reduce to:

conf = remaining_points.index(start_point)
if conf.nil?
  route << remaining_points.slice!(conf.to_i)
else
  route << remaining_points.slice!(conf.to_i)
end

OK, since the if/else are the same, it's really

conf = remaining_points.index(start_point)
route << remaining_points.slice!(conf.to_i)

And finally

route << remaining_points.slice!(remaining_points.index(start_point))

How about that?

route << remaining_points.slice!(0)
else
route << remaining_points.slice!(conf)
end
route
until remaining_points == [] do
next_point = shortest_distance(route.last, remaining_points)
route << remaining_points.slice!(remaining_points.index(next_point.fetch(:point)))
@total_distance += next_point.fetch(:distance)
end
display = {route: route, distance: @total_distance}
end

def self.shortest_distance(from, possible)
def self.shortest_distance(form, possible)
Copy link
Member

Choose a reason for hiding this comment

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

probably a typo, right? should from from ?

distances = possible.map do |point|
{point: point, distance: Map.distance_between(from, point)}
{point: point, distance: Map.distance_between(form, point)}
end
distances.sort{|a,b| a.fetch(:distance) <=> b.fetch(:distance)}.first.fetch(:point)
point = distances.sort{|a,b| a.fetch(:distance) <=> b.fetch(:distance)}.first
end
end

end
3 changes: 2 additions & 1 deletion lib/map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ def self.search(terms)
Array(Geocoder.search(terms)).first
end


def self.distance_between(first, second)
Geocoder::Calculations.distance_between(first, second)
end
end
end
6 changes: 3 additions & 3 deletions lib/place.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
require_relative "./map"

class Place
attr_accessor :name, :coordinates

attr_accessor :name, :coordinates
def self.build(name)
results = Map.search(name)
Place.new.tap do |p|
Expand All @@ -18,4 +17,5 @@ def to_s
def to_coordinates
coordinates
end
end

end
24 changes: 22 additions & 2 deletions lib/sales_person.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,35 @@
require_relative "./calculates_route"

class SalesPerson

attr_reader :cities

def initialize
@cities = []
end

def schedule_city(city)
@cities << city unless @cities.include?(city)
end

def find_city(start_name)
found = cities.select{|city| city if city.name == start_name}
found.first
end

def route
CalculatesRoute.calculate(cities)
def route(start)
city = find_city(start)
display = CalculatesRoute.calculate(cities, city)
results = {route: display.fetch(:route), time: traveling_time(display.fetch(:distance))}
z = []
x = results.fetch(:route)
y = results.fetch(:time)
z = [x, y]
return z
Copy link
Member

Choose a reason for hiding this comment

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

More ruby fun! You can remove these local variables and return:

return [results.fetch(:route), results.fetch(:time)]

end

def traveling_time(distance)
time = distance/55
end

end
42 changes: 41 additions & 1 deletion salesperson.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,50 @@
Dir["./lib/*.rb"].each {|file| require file }

require "benchmark"
require 'nokogiri'
require 'open-uri'

phil = SalesPerson.new
phil.schedule_city(Place.build("Burbank, CA"))
phil.schedule_city(Place.build("Dallas, TX"))
phil.schedule_city(Place.build("El Paso, TX"))
phil.schedule_city(Place.build("Austin, TX"))
phil.schedule_city(Place.build("Lubbock, TX"))
phil.schedule_city(Place.build("Brooklyn, NY"))
phil.schedule_city(Place.build("Griffith, Park"))
phil.schedule_city(Place.build("Berkeley, San Francisco"))
phil.schedule_city(Place.build("Los Angeles, CA"))
phil.schedule_city(Place.build("San Diego, CA"))
starting_point = "Austin, TX"
puts phil.cities
puts '------------------------'
puts phil.route(starting_point)



texas_cities = []
texas = Nokogiri::HTML(open('http://www.texas.gov/en/discover/Pages/topic.aspx?topicid=/government/localgov'))
texas.css(" .topic-subcategory-link a").map do |node|
Copy link
Member

Choose a reason for hiding this comment

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

map will return the array. so:

texas_cities = texas.css(" .topic-subcategory-link a").map do |node|
  node.content
end

In general, if you're having to declare an empty array, there's likely a better way.

texas_cities << node.content
#puts node.content
end


bench_var = [2, 3]
bench_var.each do |var|
content = SalesPerson.new
texas_cities.shuffle.take(var).each do |city|
content.schedule_city(Place.build(city))
end

Benchmark.bm do |x|
x.report do
starting_point = "Austin, Texas"
y = content.route(starting_point)
puts y
end
end
end



puts phil.route
24 changes: 13 additions & 11 deletions spec/calculates_route_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
require_relative "../lib/calculates_route"
require_relative "../lib/place"
require_relative "../lib/place.rb"

describe CalculatesRoute do
let(:dallas) {Place.build("Dallas, TX") }
let(:austin ) { Place.build("Austin, TX")}
let(:lubbock ) { Place.build("Lubbock, TX")}
let(:el_paso ) { Place.build("El Paso, TX")}
describe CalculatesRoute do
let(:austin) {Place.build("Austin, TX")}
let(:dallas) {Place.build("Dallas, TX")}
let(:el_paso) {Place.build("El Paso, TX")}
let(:lubbock) {Place.build("Lubbock, TX")}

it "should be able to calculate the route" do
start = austin
inputs = [austin, dallas, el_paso, lubbock]
expected = [austin, dallas, lubbock, el_paso]
CalculatesRoute.calculate(inputs, start).should eq(expected)

it "should calculate the route" do
points = [dallas, el_paso, austin, lubbock]
expected = [dallas, austin, lubbock, el_paso]
CalculatesRoute.calculate(points).should eq(expected)
end
end
end
15 changes: 9 additions & 6 deletions spec/map_spec.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
require_relative "../lib/place"
require_relative "../lib/map"
require 'geocoder'

describe Map do

describe Map do
describe ":search" do
it "should delegate search to geocoder" do
it "should be able to search in geocoder" do
Geocoder.should_receive(:search).with("austin, tx")
Map.search("austin, tx")
end
Expand All @@ -18,11 +19,13 @@
end

describe ":distance" do
it "should calculate distance between two sets of coordinates" do
alpha = stub
beta = stub
it "should measure the accurate distance between points" do
alpha = stub
beta = stub
Geocoder::Calculations.should_receive(:distance_between).with(alpha, beta)
Map.distance_between(alpha, beta)
end
end
end


end
46 changes: 24 additions & 22 deletions spec/place_spec.rb
Original file line number Diff line number Diff line change
@@ -1,43 +1,45 @@
require_relative "../lib/place"
require_relative "../lib/map"

describe Place do
require_relative '../lib/place'
require_relative '../lib/map'

describe Place do

it "should have a name" do
subject.should respond_to(:name)
end
it "should have a coordinates" do
subject.coordinates = [29,-95]
subject.coordinates.should eq([29,-95])
end

it "should have a coordinate" do
subject.coordinates = [-29, -95]
subject.coordinates.should eq([-29, -95])
end

describe ":build" do
let(:name) { "El Paso, TX"}
let(:name) {"El Paso, TX"}
let(:result) { stub("el paso", coordinates: [29, -95])}

it "should build from the map" do
Map.should_receive(:search).with(name).and_return(result)
Place.build(name)
end
it "should build from the map" do
Map.should_receive(:search).with(name).and_return(result)
Place.build(name)
end

it "should be place" do
Map.stub(:search).with(name).and_return(result)
Place.build(name).should be_a(Place)
end
it "should be place" do
Map.stub(:search).with(name).and_return(result)
Place.build(name).should be_a(Place)
end
end

describe "#to_s" do
it "should use the city as the to_s" do
subject.stub(:name) { "Boston" }
subject.stub(:name) {"Boston"}
subject.to_s.should eq("Boston")
end
end

describe "#to_coordinates" do
it "should delegate to_coorinates to coordinates" do
subject.stub(:coordinates) { [5,5]}
subject.to_coordinates.should eq ([5,5])
it "should use the coordinates as the to_s" do
subject.stub(:coordinates) {[29, -95]}
subject.to_coordinates.should eq([29, -95])
end
end
end


end
63 changes: 39 additions & 24 deletions spec/sales_person_spec.rb
Original file line number Diff line number Diff line change
@@ -1,30 +1,45 @@
require_relative "../lib/sales_person"
require_relative "../lib/calculates_route"
require_relative '../lib/sales_person'
require_relative '../lib/calculates_route'
require 'rspec'

describe SalesPerson do
it "should have many cities" do
city = stub
subject.schedule_city(city)
subject.cities.should include(city)
end
describe SalesPerson do
it "should be able to schedule many cities" do
city = stub
subject.schedule_city(city)
subject.cities.should include(city)
end

it "should keep the cities only scheduled once" do
city = stub
expect{
it "should not include the same city" do
city = stub
city = stub
subject.schedule_city(city)
subject.schedule_city(city)
}.to change(subject.cities,:count).by(1)
end
subject.cities.count.should eq(1)
end

it "should calculate a route via the CalculatesRoute" do
cities = [stub, stub, stub]
subject.stub(:cities) { cities }
CalculatesRoute.should_receive(:calculate).with(cities)
subject.route
end
it "should returns the route from CalculatesRoute" do
route_stub = [stub, stub]
CalculatesRoute.stub(:calculate) { route_stub }
subject.route.should eq(route_stub)
it "should find a starting point" do
cities = [stub, stub, stub]
start = stub
subject.find_city(start).should eq(start)
end
# route needs to use CalcuatesRoute.calculate method to return the correct
# arrangement of cities
it "should be able to calcuate the route via CalculatesRoute" do
start = stub("Austin, TX")
point = stub("Los Angeles, CA")
cities = [start, point]
subject.stub(:cities) {cities}
CalculatesRoute.should_receive(:calculate).with(cities, start)
#subject.route
end

it "should return the route from CalcuatesRoute" do
start = [stub]
route_stub = [stub, stub]
cities = [stub, stub, stub]
CalculatesRoute.stub(:calculate) {[start, route_stub]}
subject.route.should eq([stub, stub, stub])


end
end
end