From 0abd32fca3939437830f26a0d43a00d06d8427e7 Mon Sep 17 00:00:00 2001
From: Moritz Kobitzsch <moritz@mapbox.com>
Date: Thu, 22 Sep 2016 12:27:55 +0200
Subject: [PATCH] prefer first result

---
 features/step_definitions/trip.js               | 16 ++++++++++++++--
 features/testbot/trip.feature                   | 16 +++++++++-------
 include/engine/trip/trip_brute_force.hpp        |  8 +++++---
 include/engine/trip/trip_farthest_insertion.hpp |  2 +-
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/features/step_definitions/trip.js b/features/step_definitions/trip.js
index 57ce0795f..8de1fe7eb 100644
--- a/features/step_definitions/trip.js
+++ b/features/step_definitions/trip.js
@@ -1,6 +1,10 @@
 var util = require('util');
 
 module.exports = function () {
+    function add(a, b) {
+        return a + b;
+    }
+
     this.When(/^I plan a trip I should get$/, (table, callback) => {
         var got;
 
@@ -43,6 +47,7 @@ module.exports = function () {
                     }
 
                     var subTrips;
+                    var trip_durations;
                     if (res.statusCode === 200) {
                         if (headers.has('trips')) {
                             subTrips = json.trips.filter(t => !!t).map(t => t.legs).map(tl => Array.prototype.concat.apply([], tl.map((sl, i) => {
@@ -52,6 +57,12 @@ module.exports = function () {
                                 return toAdd;
                             })));
                         }
+                        if(headers.has('durations')) {
+                            var all_durations = json.trips.filter(t => !!t).map(t => t.legs).map(tl => Array.prototype.concat.apply([], tl.map(sl => {
+                                return sl.duration;
+                            })));
+                            trip_durations = all_durations.map( a => a.reduce(add, 0));
+                        }
                     }
 
                     var ok = true,
@@ -62,7 +73,6 @@ module.exports = function () {
                         if (si >= subTrips.length) {
                             ok = false;
                         } else {
-                            ok = false;
                             // TODO: Check all rotations of the round trip
                             for (var ni=0; ni<sub.length; ni++) {
                                 var node = this.findNodeByName(sub[ni]),
@@ -70,8 +80,8 @@ module.exports = function () {
                                 if (this.FuzzyMatch.matchLocation(outNode, node)) {
                                     encodedResult += sub[ni];
                                     extendedTarget += sub[ni];
-                                    ok = true;
                                 } else {
+                                    ok = false;
                                     encodedResult += util.format('? [%s,%s]', outNode[0], outNode[1]);
                                     extendedTarget += util.format('%s [%d,%d]', sub[ni], node.lat, node.lon);
                                 }
@@ -87,6 +97,8 @@ module.exports = function () {
                         got.trips = extendedTarget;
                     }
 
+                    got.durations = trip_durations;
+
                     for (var key in row) {
                         if (this.FuzzyMatch.match(got[key], row[key])) {
                             got[key] = row[key];
diff --git a/features/testbot/trip.feature b/features/testbot/trip.feature
index 5f3376369..33a1b4956 100644
--- a/features/testbot/trip.feature
+++ b/features/testbot/trip.feature
@@ -8,7 +8,7 @@ Feature: Basic trip planning
     Scenario: Testbot - Trip planning with less than 10 nodes
         Given the node map
             | a | b |
-            | d | c |
+            | c | d |
 
         And the ways
             | nodes |
@@ -18,8 +18,9 @@ Feature: Basic trip planning
             | da    |
 
         When I plan a trip I should get
-            | waypoints | trips |
-            | a,b,c,d   | dcba  |
+            | waypoints | trips | durations |
+            | a,b,c,d   | abcd  | 7.6       |
+            | d,b,c,a   | dbca  | 7.6       |
 
     Scenario: Testbot - Trip planning with more than 10 nodes
         Given the node map
@@ -55,8 +56,8 @@ Feature: Basic trip planning
             | k |   |   | f |
             | j | i | h | g |
             |   |   |   |   |
-            | m | n |   |   |
-            | p | o |   |   |
+            | q | m | n |   |
+            |   | p | o |   |
 
         And the ways
             | nodes |
@@ -75,12 +76,13 @@ Feature: Basic trip planning
             | mn    |
             | no    |
             | op    |
-            | pm    |
+            | pq    |
+            | qm    |
 
 
         When I plan a trip I should get
             | waypoints                       | trips              |
-            | a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p | cbalkjihgfedc,ponm |
+            | a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p | cbalkjihgfedc,mnop |
 
     # Test single node in each component #1850
     Scenario: Testbot - Trip planning with less than 10 nodes
diff --git a/include/engine/trip/trip_brute_force.hpp b/include/engine/trip/trip_brute_force.hpp
index 497c0bf6f..fcf4ebf8c 100644
--- a/include/engine/trip/trip_brute_force.hpp
+++ b/include/engine/trip/trip_brute_force.hpp
@@ -53,8 +53,7 @@ std::vector<NodeID> BruteForceTrip(const NodeIDIterator start,
     const auto component_size = std::distance(start, end);
 
     std::vector<NodeID> perm(start, end);
-    std::vector<NodeID> route;
-    route.reserve(component_size);
+    std::vector<NodeID> route = perm;
 
     EdgeWeight min_route_dist = INVALID_EDGE_WEIGHT;
 
@@ -68,7 +67,10 @@ std::vector<NodeID> BruteForceTrip(const NodeIDIterator start,
     do
     {
         const auto new_distance = ReturnDistance(dist_table, perm, min_route_dist, component_size);
-        if (new_distance <= min_route_dist)
+        // we can use `<` instead of `<=` here, since all distances are `!=` INVALID_EDGE_WEIGHT
+        // In case we really sum up to invalid edge weight for all permutations, keeping the very
+        // first one is fine too.
+        if (new_distance < min_route_dist)
         {
             min_route_dist = new_distance;
             route = perm;
diff --git a/include/engine/trip/trip_farthest_insertion.hpp b/include/engine/trip/trip_farthest_insertion.hpp
index 038386a3b..b0a6fc427 100644
--- a/include/engine/trip/trip_farthest_insertion.hpp
+++ b/include/engine/trip/trip_farthest_insertion.hpp
@@ -119,7 +119,7 @@ std::vector<NodeID> FindRoute(const std::size_t &number_of_locations,
 
                 // add the location to the current trip such that it results in the shortest total
                 // tour
-                if (insert_candidate.first >= farthest_distance)
+                if (insert_candidate.first > farthest_distance)
                 {
                     farthest_distance = insert_candidate.first;
                     next_node = *i;