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
Show file tree
Hide file tree
Changes from all 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.

28 changes: 20 additions & 8 deletions lib/calculates_route.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,34 @@
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)
distances = possible.map do |point|
{point: point, distance: Map.distance_between(from, 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
21 changes: 19 additions & 2 deletions lib/sales_person.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,32 @@
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))}
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
36 changes: 22 additions & 14 deletions spec/calculates_route_spec.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
require_relative "../lib/calculates_route"
require_relative "../lib/place"

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")}

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
require_relative "../lib/place.rb"

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 show the correct route" do
start = austin
inputs = [austin, dallas, el_paso, lubbock]
expected = [austin, dallas, lubbock, el_paso]
CalculatesRoute.calculate(inputs, start).fetch(:route).should eq(expected)
end

it "should be able to calculate the total distance" do
start = austin
inputs = [austin, dallas, el_paso, lubbock]
CalculatesRoute.calculate(inputs, start).fetch(:distance).should_not be(0)

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
city = stub
city.stub(name: "Austin, TX")
subject.schedule_city(city)
subject.find_city("Austin, TX").should eq(city)
end
it "should be able to calcuate the route via CalculatesRoute" do
start = stub(name: "Austin, TX")
point = stub(name: "Los Angeles, CA")
cities = [start, point]
subject.stub(:cities) {cities}
CalculatesRoute.should_receive(:calculate).with(cities, start)
subject.route("Austin, TX")
end

it "should return the route from CalcuatesRoute" do
route = [stub("Austin, Tx"), stub("Los Angeles, CA")]
start = stub("Austin, TX")
cities = [stub, stub, stub]
CalculatesRoute.stub(:calculate) {route}
CalculatesRoute.stub(:route).and_return(route)
subject.route.(start).should eq(route)


end
end
end