Apply suggestions from review

qgis · Oct 26, 2024 · 527ba10 · 527ba10
1 parent 1374a8a
commit 527ba10
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 14 deletions.
diff --git a/python/PyQt6/core/auto_generated/layout/qgslayoutitemnodeitem.sip.in b/python/PyQt6/core/auto_generated/layout/qgslayoutitemnodeitem.sip.in
@@ -112,13 +112,15 @@ Deselects any selected nodes.
 
 
 
-    virtual bool isValid() const;
+    virtual bool isValid() const = 0;
 %Docstring
-Can be reimplemented in subclasses. Typically a polyline is valid if it has at least 2 distinct nodes, while
-a polygon is valid if it has at least 3 distinct nodes.
+Must be reimplemented in subclasses.
+Typically a polyline is valid if it has at least 2 distinct nodes,
+while a polygon is valid if it has at least 3 distinct nodes.
 
 .. versionadded:: 3.40
 %End
+
   protected:
 
     QgsLayoutNodesItem( QgsLayout *layout );

diff --git a/python/core/auto_generated/layout/qgslayoutitemnodeitem.sip.in b/python/core/auto_generated/layout/qgslayoutitemnodeitem.sip.in
@@ -112,13 +112,15 @@ Deselects any selected nodes.
 
 
 
-    virtual bool isValid() const;
+    virtual bool isValid() const = 0;
 %Docstring
-Can be reimplemented in subclasses. Typically a polyline is valid if it has at least 2 distinct nodes, while
-a polygon is valid if it has at least 3 distinct nodes.
+Must be reimplemented in subclasses.
+Typically a polyline is valid if it has at least 2 distinct nodes,
+while a polygon is valid if it has at least 3 distinct nodes.
 
 .. versionadded:: 3.40
 %End
+
   protected:
 
     QgsLayoutNodesItem( QgsLayout *layout );

diff --git a/src/core/layout/qgslayoutitemnodeitem.h b/src/core/layout/qgslayoutitemnodeitem.h
@@ -115,12 +115,13 @@ class CORE_EXPORT QgsLayoutNodesItem: public QgsLayoutItem
 
 
     /**
-     * Can be reimplemented in subclasses. Typically a polyline is valid if it has at least 2 distinct nodes, while
-     * a polygon is valid if it has at least 3 distinct nodes.
+     * Must be reimplemented in subclasses.
+     * Typically a polyline is valid if it has at least 2 distinct nodes,
+     * while a polygon is valid if it has at least 3 distinct nodes.
      *
      * \since QGIS 3.40
      */
-    virtual bool isValid() const { return true; };
+    virtual bool isValid() const = 0;
 
   protected:
 

diff --git a/src/core/layout/qgslayoutitempolygon.cpp b/src/core/layout/qgslayoutitempolygon.cpp
@@ -132,6 +132,7 @@ QgsGeometry QgsLayoutItemPolygon::clipPath() const
 
 bool QgsLayoutItemPolygon::isValid() const
 {
+  // A Polygon is valid if it has at least 3 unique points
   QList<QPointF> uniquePoints;
   int seen = 0;
   for ( QPointF point : mPolygon )

diff --git a/src/core/layout/qgslayoutitempolyline.cpp b/src/core/layout/qgslayoutitempolyline.cpp
@@ -341,6 +341,7 @@ QPainterPath QgsLayoutItemPolyline::shape() const
 
 bool QgsLayoutItemPolyline::isValid() const
 {
+  // A Polyline is valid if it has at least 2 unique points
   QList<QPointF> uniquePoints;
   int seen = 0;
   for ( QPointF point : mPolygon )

diff --git a/src/gui/layout/qgslayoutviewtooladdnodeitem.cpp b/src/gui/layout/qgslayoutviewtooladdnodeitem.cpp
@@ -69,23 +69,22 @@ void QgsLayoutViewToolAddNodeItem::layoutPressEvent( QgsLayoutViewMouseEvent *ev
     // last (temporary) point is removed
     mPolygon.remove( mPolygon.count() - 1 );
 
-    QgsLayoutItem *item = QgsGui::layoutItemGuiRegistry()->createItem( mItemMetadataId, layout() );
+    std::unique_ptr< QgsLayoutItem > item( QgsGui::layoutItemGuiRegistry()->createItem( mItemMetadataId, layout() ) );
     if ( !item )
       return;
 
-    if ( QgsLayoutNodesItem *nodesItem = qobject_cast< QgsLayoutNodesItem * >( item ) )
+    if ( QgsLayoutNodesItem *nodesItem = qobject_cast< QgsLayoutNodesItem * >( item.get() ) )
     {
       nodesItem->setNodes( mPolygon );
       if ( !nodesItem->isValid() )
       {
-        nodesItem->deleteLater();
         mRubberBand.reset();
         return;
       }
     }
 
-    layout()->addLayoutItem( item );
-    layout()->setSelectedItem( item );
+    layout()->addLayoutItem( item.get() );
+    layout()->setSelectedItem( item.release() );
     emit createdItem();
   }
   else