← Back to team overview

kicad-developers team mailing list archive

[PATCH v2] Rewrite line merging without static vector and sort

 

The static vector is a massive code smell, and as we're only interested in
the most extreme elements, we can simply use std::min and std::max with
custom comparison functions instead.
---
 eeschema/sch_line.cpp | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 72efb5c..376a56f 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -270,17 +270,6 @@ void SCH_LINE::Rotate( wxPoint aPosition )
 
 
 /*
- * helper sort function, used by MergeOverlap
- * sorts ref and test by x values, or (for same x values) by y values
- */
-bool sort_by_ends_position(const wxPoint * ref, const wxPoint * tst )
-{
-    if( ref->x == tst->x )
-        return ref->y < tst->y;
-    return ref->x < tst->x;
-}
-
-/*
  * MergeOverlap try to merge 2 lines that are colinear.
  * this function expects these 2 lines have at least a common end
  */
@@ -346,16 +335,18 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     // for horizontal segments the uppermost and the lowest point
     if( colinear )
     {
-        static std::vector <wxPoint*> candidates;
-        candidates.clear();
-        candidates.push_back( &m_start );
-        candidates.push_back( &m_end );
-        candidates.push_back( &aLine->m_start );
-        candidates.push_back( &aLine->m_end );
-        sort( candidates.begin(), candidates.end(), sort_by_ends_position );
-        wxPoint tmp = *candidates[3];
-        m_start = *candidates[0];
-        m_end = tmp;
+        auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
+        {
+            if( lhs.x == rhs.x )
+                return lhs.y < rhs.y;
+            return lhs.x < rhs.y;
+        };
+
+        wxPoint top_left = std::min( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
+        wxPoint bottom_right = std::max( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
+
+        m_start = top_left;
+        m_end = bottom_right;
         return true;
     }
     return false;

Follow ups

References