v0.2.49..v0.2.50 changeset Way.cpp

diff --git a/hoot-core/src/main/cpp/hoot/core/elements/Way.cpp b/hoot-core/src/main/cpp/hoot/core/elements/Way.cpp
index 6d00fab..87f8749 100644
--- a/hoot-core/src/main/cpp/hoot/core/elements/Way.cpp
+++ b/hoot-core/src/main/cpp/hoot/core/elements/Way.cpp
@@ -32,6 +32,7 @@
 #include <hoot/core/elements/ElementConverter.h>
 #include <hoot/core/elements/Node.h>
 #include <hoot/core/util/GeometryUtils.h>
+#include <hoot/core/util/CollectionUtils.h>
 
 // Geos
 #include <geos/geom/CoordinateSequenceFactory.h>
@@ -72,6 +73,13 @@ void Way::addNode(long id)
   _makeWritable();
   _wayData->addNode(id);
   _postGeometryChange();
+
+//  // for debugging only; SLOW
+//  if (_nodeIdsAreDuplicated(getNodeIds()))
+//  {
+//    LOG_VARE(getNodeIds());
+//    throw IllegalArgumentException("Duplicate way node IDs: addNode");
+//  }
 }
 
 void Way::insertNode(long index, long id)
@@ -80,11 +88,57 @@ void Way::insertNode(long index, long id)
   _makeWritable();
   _wayData->insertNode(index, id);
   _postGeometryChange();
+
+//  // for debugging only; SLOW
+//  if (_nodeIdsAreDuplicated(getNodeIds()))
+//  {
+//    LOG_VARE(getNodeIds());
+//    throw IllegalArgumentException("Duplicate way node IDs: insertNode");
+//  }
 }
 
 void Way::addNodes(const vector<long>& ids)
 {
   addNodes(ids.begin(), ids.end());
+
+  //  // for debugging only; SLOW
+  //  if (_nodeIdsAreDuplicated(getNodeIds()))
+  //  {
+  //    LOG_VARE(getNodeIds());
+  //    throw IllegalArgumentException("Duplicate way node IDs: addNodes");
+  //  }
+}
+
+bool Way::_nodeIdsAreDuplicated(const vector<long>& ids) const
+{
+  if (ids.size() == 1)
+  {
+    return false;
+  }
+
+  LOG_VART(ids);
+
+  QList<long> idsCopy;
+  for (size_t i = 0; i < ids.size(); i++)
+  {
+    const long id = ids[i];
+    LOG_VART(id);
+    LOG_VART(i);
+    LOG_VART(idsCopy.lastIndexOf(id));
+    if (!idsCopy.contains(id))
+    {
+      idsCopy.append(id);
+    }
+    else if (i == ids.size() - 1 && idsCopy.lastIndexOf(id) != 0)
+    {
+      return true;
+    }
+    else if (i != ids.size() - 1)
+    {
+      return true;
+    }
+  }
+  return false;
 }
 
 void Way::clear()
@@ -187,6 +241,13 @@ void Way::setNodes(const vector<long>& newNodes)
   _wayData->getNodeIds() = newNodes;
 
   _postGeometryChange();
+
+  //  // for debugging only; SLOW
+  //  if (_nodeIdsAreDuplicated(getNodeIds()))
+  //  {
+  //    LOG_VARE(getNodeIds());
+  //    throw IllegalArgumentException("Duplicate way node IDs: setNodes");
+  //  }
 }
 
 int Way::getNodeIndex(long nodeId) const
@@ -267,6 +328,8 @@ void Way::_makeWritable()
 
 void Way::removeNode(long id)
 {
+  LOG_TRACE("Removing node: " << id << " in way: " << getId() << "...");
+
   std::vector<long>& nodes = _wayData->getNodeIds();
   size_t newCount = 0;
 
@@ -283,16 +346,36 @@ void Way::removeNode(long id)
   nodes.resize(newCount);
 }
 
+bool Way::isFirstAndLastNode(const long nodeId) const
+{
+  return nodeId == getFirstNodeId() && nodeId == getLastNodeId();
+}
+
 void Way::replaceNode(long oldId, long newId)
 {
-  LOG_TRACE("Replacing node: " << oldId << " with: " << newId << "...");
+  // Note: Noticed that duplicate way nodes were being introduced here when called by
+  // MergeNearbyNodes. Initially made some attempts in here to clean that up, but it introduced
+  // unwanted logic and runtime complexity. Ultimately since the problem was only seen with
+  // MergeNearbyNodes, it was updated to remove the duplicated way nodes at the end of its run and
+  // this logic was left alone. The argument can be made that this method allows you to replace
+  // whatever is desired, but its the caller's responsbility to make sure the result of the
+  // replacement is valid.
+
+  if (oldId == newId)
+  {
+    return;
+  }
+
+  LOG_TRACE(
+    "Replacing node: " << oldId << " with: " << newId << " in way: " << getId() << "...");
 
   const vector<long>& ids = getNodeIds();
 
   bool change = false;
   for (size_t i = 0; i < ids.size(); i++)
   {
-    if (ids[i] == oldId)
+    const long id = ids[i];
+    if (id == oldId)
     {
       change = true;
     }
@@ -300,8 +383,14 @@ void Way::replaceNode(long oldId, long newId)
 
   if (change)
   {
+    LOG_TRACE("IDs before replacement: " << getNodeIds());
+
+    // for debugging only; see _nodeIdsAreDuplicated call below
+    //const vector<long> oldIdsCopy = ids;
+
     _preGeometryChange();
     _makeWritable();
+
     vector<long>& newIds = _wayData->getNodeIds();
     for (size_t i = 0; i < newIds.size(); i++)
     {
@@ -310,7 +399,21 @@ void Way::replaceNode(long oldId, long newId)
         newIds[i] = newId;
       }
     }
+
     _postGeometryChange();
+
+    LOG_TRACE("IDs after replacement: " << getNodeIds());
+
+//    // for debugging only; SLOW
+//    if (_nodeIdsAreDuplicated(getNodeIds()))
+//    {
+//      LOG_ERROR(
+//        "Attempting to replace node: " << oldId << " with: " << newId << " in way: " << getId() <<
+//        "...");
+//      LOG_ERROR("Original IDs: " << oldIdsCopy);
+//      LOG_ERROR("New IDs: " << getNodeIds());
+//      throw IllegalArgumentException("Duplicate way node IDs: replaceNode");
+//    }
   }
 }
 
@@ -350,12 +453,12 @@ QString Way::toString() const
 
 bool Way::isFirstLastNodeIdentical() const
 {
-  if ( getNodeCount() < 2 )
+  if (getNodeCount() < 2)
   {
     return false;
   }
 
-  return ( getFirstNodeId() == getLastNodeId() );
+  return (getFirstNodeId() == getLastNodeId());
 }
 
 bool Way::isClosedArea() const
@@ -376,4 +479,9 @@ long Way::getPid(long p, long c)
   else                                  return WayData::PID_EMPTY;
 }
 
+bool Way::hasSameNodeIds(const Way& other) const
+{
+  return getNodeIds() == other.getNodeIds();
+}
+
 }