No changes, just slightly simplify the tool insertion code.

Added a helper DoInsertNewTool() function to avoid code duplication and ensure
that we never forget to delete a new tool if inserting it into the toolbar
failed.

Also explicitly document that the tool passed to the public InsertTool() is
not owned by the toolbar and so must be deleted by the caller if its insertion
failed.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@62846 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/include/wx/tbarbase.h b/include/wx/tbarbase.h
index 14d03e032b222908dd81843c107452a66f4b39ba..ba6a73e489f68446c8942da93e35d56d8f103817 100644 (file)
--- a/include/wx/tbarbase.h
+++ b/include/wx/tbarbase.h
@@ -608,6 +608,17 @@ protected:
     // make the size of the buttons big enough to fit the largest bitmap size
     void AdjustToolBitmapSize();
 
     // make the size of the buttons big enough to fit the largest bitmap size
     void AdjustToolBitmapSize();
 

+    // calls InsertTool() and deletes the tool if inserting it failed
+    wxToolBarToolBase *DoInsertNewTool(size_t pos, wxToolBarToolBase *tool)
+    {
+        if ( !InsertTool(pos, tool) )
+        {
+            delete tool;
+            return NULL;
+        }
+
+        return tool;
+    }

 
     // the list of all our tools
     wxToolBarToolsList m_tools;
 
     // the list of all our tools
     wxToolBarToolsList m_tools;

diff --git a/interface/wx/toolbar.h b/interface/wx/toolbar.h
index 93b844b26f30e470ede8f87b591c549fc7c79ff7..36cbd2eb605d83cac846cf87cc6e8a1528b382ca 100644 (file)
--- a/interface/wx/toolbar.h
+++ b/interface/wx/toolbar.h
@@ -533,6 +533,10 @@ public:
         You must call Realize() for the change to take place.
 
         @see AddTool(), InsertControl(), InsertSeparator()
         You must call Realize() for the change to take place.
 
         @see AddTool(), InsertControl(), InsertSeparator()

+
+        @return The newly inserted tool or @NULL on failure. Notice that with
+            the overload taking @a tool parameter the caller is responsible for
+            deleting the tool in the latter case.

     */
     wxToolBarToolBase* InsertTool(size_t pos, int toolId,
                                   const wxBitmap& bitmap1,
     */
     wxToolBarToolBase* InsertTool(size_t pos, int toolId,
                                   const wxBitmap& bitmap1,

diff --git a/src/common/tbarbase.cpp b/src/common/tbarbase.cpp
index abc7670961e7465b318842ac661d871da3a2c8a1..37f38fa16984a6526419757bf86bcc9741c6fdb6 100644 (file)
--- a/src/common/tbarbase.cpp
+++ b/src/common/tbarbase.cpp
@@ -180,17 +180,8 @@ wxToolBarToolBase *wxToolBarBase::InsertTool(size_t pos,
     wxCHECK_MSG( pos <= GetToolsCount(), NULL,
                  wxT("invalid position in wxToolBar::InsertTool()") );
 
     wxCHECK_MSG( pos <= GetToolsCount(), NULL,
                  wxT("invalid position in wxToolBar::InsertTool()") );
 

-    wxToolBarToolBase *tool = CreateTool(id, label, bitmap, bmpDisabled, kind,
-                                         clientData, shortHelp, longHelp);
-
-    if ( !InsertTool(pos, tool) )
-    {
-        delete tool;
-
-        return NULL;
-    }
-
-    return tool;
+    return DoInsertNewTool(pos, CreateTool(id, label, bitmap, bmpDisabled, kind,
+                                           clientData, shortHelp, longHelp));

 }
 
 wxToolBarToolBase *wxToolBarBase::AddTool(wxToolBarToolBase *tool)
 }
 
 wxToolBarToolBase *wxToolBarBase::AddTool(wxToolBarToolBase *tool)


@@ -232,19 +223,7 @@ wxToolBarBase::InsertControl(size_t pos,
     wxCHECK_MSG( control->GetParent() == this, NULL,
                  wxT("control must have toolbar as parent") );
 
     wxCHECK_MSG( control->GetParent() == this, NULL,
                  wxT("control must have toolbar as parent") );
 

-    wxCHECK_MSG( pos <= GetToolsCount(), NULL,
-                 wxT("invalid position in wxToolBar::InsertControl()") );
-
-    wxToolBarToolBase *tool = CreateTool(control, label);
-
-    if ( !InsertTool(pos, tool) )
-    {
-        delete tool;
-
-        return NULL;
-    }
-
-    return tool;
+    return DoInsertNewTool(pos, CreateTool(control, label));

 }
 
 wxControl *wxToolBarBase::FindControl( int id )
 }
 
 wxControl *wxToolBarBase::FindControl( int id )


@@ -280,25 +259,11 @@ wxToolBarToolBase *wxToolBarBase::AddSeparator()
 
 wxToolBarToolBase *wxToolBarBase::InsertSeparator(size_t pos)
 {
 
 wxToolBarToolBase *wxToolBarBase::InsertSeparator(size_t pos)
 {

-    wxCHECK_MSG( pos <= GetToolsCount(), NULL,
-                 wxT("invalid position in wxToolBar::InsertSeparator()") );
-
-    wxToolBarToolBase *tool = CreateTool(wxID_SEPARATOR,
-                                         wxEmptyString,
-                                         wxNullBitmap, wxNullBitmap,
-                                         wxITEM_SEPARATOR, NULL,
-                                         wxEmptyString, wxEmptyString);
-
-    if ( !tool || !DoInsertTool(pos, tool) )
-    {
-        delete tool;
-
-        return NULL;
-    }
-
-    m_tools.Insert(pos, tool);
-
-    return tool;
+    return DoInsertNewTool(pos, CreateTool(wxID_SEPARATOR,
+                                           wxEmptyString,
+                                           wxNullBitmap, wxNullBitmap,
+                                           wxITEM_SEPARATOR, NULL,
+                                           wxEmptyString, wxEmptyString));

 }
 
 wxToolBarToolBase *wxToolBarBase::RemoveTool(int id)
 }
 
 wxToolBarToolBase *wxToolBarBase::RemoveTool(int id)