Refactor|libshell: Use reference-counted Action instances

skyjake · Feb 3, 2014 · eb973f9 · eb973f9
1 parent aae0f35
commit eb973f9
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 24 deletions.
diff --git a/doomsday/libshell/include/de/shell/action.h b/doomsday/libshell/include/de/shell/action.h
@@ -42,8 +42,6 @@ class Action : public QObject, public de::Action
 
     Action(KeyEvent const &event, QObject *target = 0, char const *slot = 0);
 
-    ~Action();
-
     void setLabel(String const &label);
 
     String label() const;
@@ -59,11 +57,12 @@ class Action : public QObject, public de::Action
 
     void trigger();
 
-    Action *duplicate() const;
-
 signals:
     void triggered();
 
+protected:
+    ~Action();
+
 private:
     KeyEvent _event;
     String _label;

diff --git a/doomsday/libshell/include/de/shell/menuwidget.h b/doomsday/libshell/include/de/shell/menuwidget.h
@@ -64,7 +64,7 @@ class LIBSHELL_PUBLIC MenuWidget : public TextWidget
      * @param action  Action to add as a shortcut for triggering the item.
      * @param shortcutLabel  Label to show, representing the action shortcut to the user.
      */
-    void appendItem(Action *action, String const &shortcutLabel = "");
+    void appendItem(RefArg<Action> action, String const &shortcutLabel = "");
 
     /**
      * Inserts an item into the menu.
@@ -73,7 +73,7 @@ class LIBSHELL_PUBLIC MenuWidget : public TextWidget
      * @param action  Action to add as a shortcut for triggering the item.
      * @param shortcutLabel  Label to show, representing the action shortcut to the user.
      */
-    void insertItem(int pos, Action *action, String const &shortcutLabel = "");
+    void insertItem(int pos, RefArg<Action> action, String const &shortcutLabel = "");
 
     void appendSeparator();
 

diff --git a/doomsday/libshell/include/de/shell/textwidget.h b/doomsday/libshell/include/de/shell/textwidget.h
@@ -108,7 +108,7 @@ class LIBSHELL_PUBLIC TextWidget : public QObject, public Widget
      *
      * @param action  Action instance. Ownership taken.
      */
-    void addAction(Action *action);
+    void addAction(RefArg<Action> action);
 
     /**
      * Removes an action from the widget.

diff --git a/doomsday/libshell/src/action.cpp b/doomsday/libshell/src/action.cpp
@@ -80,10 +80,5 @@ void Action::trigger()
     emit triggered();
 }
 
-Action *Action::duplicate() const
-{
-    return new Action(_label, _event, _target, _slot);
-}
-
 } // namespace shell
 } // namespace de
diff --git a/doomsday/libshell/src/menuwidget.cpp b/doomsday/libshell/src/menuwidget.cpp
@@ -42,8 +42,23 @@ DENG2_PIMPL(MenuWidget)
         String shortcutLabel;
         bool separatorAfter;
 
-        Item() : action(0), separatorAfter(false)
-        {}
+        Item() : action(0), separatorAfter(false) {}
+
+        Item(Item const &other)
+            : action(holdRef(other.action))
+            , shortcutLabel(other.shortcutLabel)
+            , separatorAfter(other.separatorAfter) {}
+
+        ~Item() {
+            releaseRef(action);
+        }
+
+        Item &operator = (Item const &other) {
+            changeRef(action, other.action);
+            shortcutLabel = other.shortcutLabel;
+            separatorAfter = other.separatorAfter;
+            return *this;
+        }
     };
 
     QList<Item> items;
@@ -74,7 +89,6 @@ DENG2_PIMPL(MenuWidget)
         foreach(Item i, items)
         {
             self.removeAction(*i.action);
-            delete i.action;
         }
         items.clear();
         updateSize();
@@ -103,7 +117,6 @@ DENG2_PIMPL(MenuWidget)
     void removeItem(int pos)
     {
         self.removeAction(*items[pos].action);
-        delete items[pos].action;
         items.removeAt(pos);
         updateSize();
     }
@@ -135,10 +148,10 @@ int MenuWidget::itemCount() const
     return d->items.size();
 }
 
-void MenuWidget::appendItem(Action *action, String const &shortcutLabel)
+void MenuWidget::appendItem(RefArg<Action> action, String const &shortcutLabel)
 {
     Instance::Item item;
-    item.action = action;
+    item.action = action.holdRef();
     item.shortcutLabel = shortcutLabel;
     d->items.append(item);
     d->updateSize();
@@ -156,10 +169,10 @@ void MenuWidget::appendSeparator()
     redraw();
 }
 
-void MenuWidget::insertItem(int pos, Action *action, String const &shortcutLabel)
+void MenuWidget::insertItem(int pos, RefArg<Action> action, String const &shortcutLabel)
 {
     Instance::Item item;
-    item.action = action;
+    item.action = action.holdRef();
     item.shortcutLabel = shortcutLabel;
     d->items.insert(pos, item);
     d->updateSize();

diff --git a/doomsday/libshell/src/textwidget.cpp b/doomsday/libshell/src/textwidget.cpp
@@ -36,7 +36,19 @@ DENG2_PIMPL_NOREF(TextWidget)
     ~Instance()
     {
         delete rule;
-        foreach(Action *act, actions) delete act;
+        foreach(Action *act, actions) releaseRef(act);
+    }
+
+    void removeAction(Action &action)
+    {
+        for(int i = actions.size() - 1; i >= 0; --i)
+        {
+            if(actions.at(i) == &action)
+            {
+                releaseRef(actions[i]);
+                actions.removeAt(i);
+            }
+        }
     }
 
     /**
@@ -123,14 +135,14 @@ Vector2i TextWidget::cursorPosition() const
                     rule().top().valuei());
 }
 
-void TextWidget::addAction(Action *action)
+void TextWidget::addAction(RefArg<Action> action)
 {
-    d->actions.append(action);
+    d->actions.append(action.holdRef());
 }
 
 void TextWidget::removeAction(Action &action)
 {
-    d->actions.removeAll(&action);
+    d->removeAction(action);
 }
 
 bool TextWidget::handleEvent(Event const &event)