Don't eliminate indirection for CommandListEntry. Explain why. Use unique_ptr

src/commands/CommandManager.cpp

@@ -415,7 +415,7 @@ void CommandManager::PurgeData()
    // mCommandList contains pointers to CommandListEntrys
    // mMenuBarList contains MenuBarListEntrys.
    // mSubMenuList contains SubMenuListEntrys
-   WX_CLEAR_ARRAY( mCommandList );
+   mCommandList.clear();
    mMenuBarList.clear();
    mSubMenuList.clear();
 
@@ -791,74 +791,80 @@ CommandListEntry *CommandManager::NewIdentifier(const wxString & name,
 }
 
 CommandListEntry *CommandManager::NewIdentifier(const wxString & name,
-                                                const wxString & label,
+   const wxString & label,
-                                                const wxString & accel,
+   const wxString & accel,
-                                                wxMenu *menu,
+   wxMenu *menu,
-                                                const CommandFunctorPointer &callback,
+   const CommandFunctorPointer &callback,
-                                                bool multi,
+   bool multi,
-                                                int index,
+   int index,
-                                                int count)
+   int count)
 {
-   CommandListEntry *entry = new CommandListEntry;
+   {
+      auto entry = std::make_unique<CommandListEntry>();
 
-   wxString labelPrefix;
+      wxString labelPrefix;
-   if (!mSubMenuList.empty()) {
+      if (!mSubMenuList.empty()) {
-      labelPrefix = mSubMenuList.back().name;
+         labelPrefix = mSubMenuList.back().name;
-   }
+      }
 
-   // wxMac 2.5 and higher will do special things with the
+      // wxMac 2.5 and higher will do special things with the
-   // Preferences, Exit (Quit), and About menu items,
+      // Preferences, Exit (Quit), and About menu items,
-   // if we give them the right IDs.
+      // if we give them the right IDs.
-   // Otherwise we just pick increasing ID numbers for each NEW
+      // Otherwise we just pick increasing ID numbers for each NEW
-   // command.  Note that the name string we are comparing
+      // command.  Note that the name string we are comparing
-   // ("About", "Preferences") is the internal command name
+      // ("About", "Preferences") is the internal command name
-   // (untranslated), not the label that actually appears in the
+      // (untranslated), not the label that actually appears in the
-   // menu (which might be translated).
+      // menu (which might be translated).
 
-   mCurrentID = NextIdentifier(mCurrentID);
+      mCurrentID = NextIdentifier(mCurrentID);
-   entry->id = mCurrentID;
+      entry->id = mCurrentID;
 
 #if defined(__WXMAC__)
-   if (name == wxT("Preferences"))
+      if (name == wxT("Preferences"))
-      entry->id = wxID_PREFERENCES;
+         entry->id = wxID_PREFERENCES;
-   else if (name == wxT("Exit"))
+      else if (name == wxT("Exit"))
-      entry->id = wxID_EXIT;
+         entry->id = wxID_EXIT;
-   else if (name == wxT("About"))
+      else if (name == wxT("About"))
-      entry->id = wxID_ABOUT;
+         entry->id = wxID_ABOUT;
 #endif
 
-   entry->name = name;
+      entry->name = name;
-   entry->label = label;
+      entry->label = label;
-   entry->key = KeyStringNormalize(accel.BeforeFirst(wxT('\t')));
+      entry->key = KeyStringNormalize(accel.BeforeFirst(wxT('\t')));
-   entry->defaultKey = entry->key;
+      entry->defaultKey = entry->key;
-   entry->labelPrefix = labelPrefix;
+      entry->labelPrefix = labelPrefix;
-   entry->labelTop = wxMenuItem::GetLabelText(mCurrentMenuName);
+      entry->labelTop = wxMenuItem::GetLabelText(mCurrentMenuName);
-   entry->menu = menu;
+      entry->menu = menu;
-   entry->callback = callback;
+      entry->callback = callback;
-   entry->multi = multi;
+      entry->multi = multi;
-   entry->index = index;
+      entry->index = index;
-   entry->count = count;
+      entry->count = count;
-   entry->flags = mDefaultFlags;
+      entry->flags = mDefaultFlags;
-   entry->mask = mDefaultMask;
+      entry->mask = mDefaultMask;
-   entry->enabled = true;
+      entry->enabled = true;
-   entry->skipKeydown = (accel.Find(wxT("\tskipKeydown")) != wxNOT_FOUND);
+      entry->skipKeydown = (accel.Find(wxT("\tskipKeydown")) != wxNOT_FOUND);
-   entry->wantKeyup = (accel.Find(wxT("\twantKeyup")) != wxNOT_FOUND) || entry->skipKeydown;
+      entry->wantKeyup = (accel.Find(wxT("\twantKeyup")) != wxNOT_FOUND) || entry->skipKeydown;
-   entry->isGlobal = false;
+      entry->isGlobal = false;
 
-   // For key bindings for commands with a list, such as effects,
+      // For key bindings for commands with a list, such as effects,
-   // the name in prefs is the category name plus the effect name.
+      // the name in prefs is the category name plus the effect name.
-   if (multi) {
+      if (multi) {
-      entry->name = wxString::Format( wxT("%s:%s"), name.c_str(), label.c_str() );
+         entry->name = wxString::Format(wxT("%s:%s"), name.c_str(), label.c_str());
+      }
+
+      // Key from preferences overridse the default key given
+      gPrefs->SetPath(wxT("/NewKeys"));
+      if (gPrefs->HasEntry(entry->name)) {
+         entry->key = KeyStringNormalize(gPrefs->Read(entry->name, entry->key));
+      }
+      gPrefs->SetPath(wxT("/"));
+
+      mCommandList.push_back(std::move(entry));
+      // Don't use the variable entry eny more!
    }
 
-   // Key from preferences overridse the default key given
+   // New variable
-   gPrefs->SetPath(wxT("/NewKeys"));
+   CommandListEntry *entry = &*mCommandList.back();
-   if (gPrefs->HasEntry(entry->name)) {
-      entry->key = KeyStringNormalize(gPrefs->Read(entry->name, entry->key));
-   }
-   gPrefs->SetPath(wxT("/"));
-
-   mCommandList.Add(entry);
    mCommandIDHash[entry->id] = entry;
 
 #if defined(__WXDEBUG__)
@@ -965,8 +971,7 @@ void CommandManager::EnableUsingFlags(wxUint32 flags, wxUint32 mask)
 {
    unsigned int i;
 
-   for(i=0; i<mCommandList.GetCount(); i++) {
+   for(const auto &entry : mCommandList) {
-      CommandListEntry *entry = mCommandList[i];
       if (entry->multi && entry->index != 0)
          continue;
 
@@ -974,7 +979,7 @@ void CommandManager::EnableUsingFlags(wxUint32 flags, wxUint32 mask)
       if (combinedMask) {
          bool enable = ((flags & combinedMask) ==
                         (entry->flags & combinedMask));
-         Enable(entry, enable);
+         Enable(entry.get(), enable);
       }
    }
 }
@@ -1020,7 +1025,7 @@ void CommandManager::SetKeyFromName(wxString name, wxString key)
 
 void CommandManager::SetKeyFromIndex(int i, wxString key)
 {
-   CommandListEntry *entry = mCommandList[i];
+   const auto &entry = mCommandList[i];
    entry->key = KeyStringNormalize(key);
 }
 
@@ -1154,13 +1159,13 @@ bool CommandManager::HandleTextualCommand(wxString & Str, wxUint32 flags, wxUint
    unsigned int i;
 
    // Linear search for now...
-   for (i = 0; i < mCommandList.GetCount(); i++)
+   for (const auto &entry : mCommandList)
    {
-      if (!mCommandList[i]->multi)
+      if (!entry->multi)
       {
-         if( Str.IsSameAs( mCommandList[i]->name ))
+         if( Str.IsSameAs( entry->name ))
          {
-            return HandleCommandEntry( mCommandList[i], flags, mask);
+            return HandleCommandEntry( entry.get(), flags, mask);
          }
       }
    }
@@ -1192,10 +1197,8 @@ void CommandManager::GetCategories(wxArrayString &cats)
 {
    cats.Clear();
 
-   size_t cnt = mCommandList.GetCount();
+   for (const auto &entry : mCommandList) {
-
+      wxString cat = entry->labelTop;
-   for (size_t i = 0; i < cnt; i++) {
-      wxString cat = mCommandList[i]->labelTop;
       if (cats.Index(cat) == wxNOT_FOUND) {
          cats.Add(cat);
       }
@@ -1224,26 +1227,22 @@ void CommandManager::GetCategories(wxArrayString &cats)
 void CommandManager::GetAllCommandNames(wxArrayString &names,
                                         bool includeMultis)
 {
-   unsigned int i;
+   for(const auto &entry : mCommandList) {
-
+      if (!entry->multi)
-   for(i=0; i<mCommandList.GetCount(); i++) {
+         names.Add(entry->name);
-      if (!mCommandList[i]->multi)
-         names.Add(mCommandList[i]->name);
       else if( includeMultis )
-         names.Add(mCommandList[i]->name + wxT(":")/*+ mCommandList[i]->label*/);
+         names.Add(entry->name + wxT(":")/*+ mCommandList[i]->label*/);
    }
 }
 
 void CommandManager::GetAllCommandLabels(wxArrayString &names,
                                         bool includeMultis)
 {
-   unsigned int i;
+   for(const auto &entry : mCommandList) {
-
+      if (!entry->multi)
-   for(i=0; i<mCommandList.GetCount(); i++) {
+         names.Add(entry->label);
-      if (!mCommandList[i]->multi)
-         names.Add(mCommandList[i]->label);
       else if( includeMultis )
-         names.Add(mCommandList[i]->label);
+         names.Add(entry->label);
    }
 }
 
@@ -1258,29 +1257,27 @@ void CommandManager::GetAllCommandData(
 #endif
    bool includeMultis)
 {
-   unsigned int i;
+   for(const auto &entry : mCommandList) {
-
+      if (!entry->multi)
-   for(i=0; i<mCommandList.GetCount(); i++) {
-      if (!mCommandList[i]->multi)
       {
-         names.Add(mCommandList[i]->name);
+         names.Add(entry->name);
-         keys.Add(mCommandList[i]->key);
+         keys.Add(entry->key);
-         default_keys.Add( mCommandList[i]->defaultKey);
+         default_keys.Add(entry->defaultKey);
-         labels.Add(mCommandList[i]->label);
+         labels.Add(entry->label);
-         categories.Add(mCommandList[i]->labelTop);
+         categories.Add(entry->labelTop);
 #if defined(EXPERIMENTAL_KEY_VIEW)
-         prefixes.Add(mCommandList[i]->labelPrefix);
+         prefixes.Add(entry->labelPrefix);
 #endif
       }
       else if( includeMultis )
       {
-         names.Add(mCommandList[i]->name);
+         names.Add(entry->name);
-         keys.Add(mCommandList[i]->key);
+         keys.Add(entry->key);
-         default_keys.Add( mCommandList[i]->defaultKey);
+         default_keys.Add(entry->defaultKey);
-         labels.Add(mCommandList[i]->label);
+         labels.Add(entry->label);
-         categories.Add(mCommandList[i]->labelTop);
+         categories.Add(entry->labelTop);
 #if defined(EXPERIMENTAL_KEY_VIEW)
-         prefixes.Add(mCommandList[i]->labelPrefix);
+         prefixes.Add(entry->labelPrefix);
 #endif
       }
    }
@@ -1389,19 +1386,17 @@ XMLTagHandler *CommandManager::HandleXMLChild(const wxChar * WXUNUSED(tag))
 
 void CommandManager::WriteXML(XMLWriter &xmlFile)
 {
-   unsigned int j;
-
    xmlFile.StartTag(wxT("audacitykeyboard"));
    xmlFile.WriteAttr(wxT("audacityversion"), AUDACITY_VERSION_STRING);
 
-   for(j=0; j<mCommandList.GetCount(); j++) {
+   for(const auto &entry : mCommandList) {
-      wxString label = mCommandList[j]->label;
+      wxString label = entry->label;
       label = wxMenuItem::GetLabelText(label.BeforeFirst(wxT('\t')));
 
       xmlFile.StartTag(wxT("command"));
-      xmlFile.WriteAttr(wxT("name"), mCommandList[j]->name);
+      xmlFile.WriteAttr(wxT("name"), entry->name);
       xmlFile.WriteAttr(wxT("label"), label);
-      xmlFile.WriteAttr(wxT("key"), mCommandList[j]->key);
+      xmlFile.WriteAttr(wxT("key"), entry->key);
       xmlFile.EndTag(wxT("command"));
    }
 
@@ -1450,7 +1445,7 @@ void CommandManager::SetCommandFlags(wxUint32 flags, wxUint32 mask, ...)
 #if defined(__WXDEBUG__)
 void CommandManager::CheckDups()
 {
-   int cnt = mCommandList.GetCount();
+   int cnt = mCommandList.size();
    for (size_t j = 0;  (int)j < cnt; j++) {
       if (mCommandList[j]->key.IsEmpty()) {
          continue;

src/commands/CommandManager.h

@@ -79,7 +79,10 @@ struct CommandListEntry
 
 using MenuBarList = std::vector < MenuBarListEntry >;
 using SubMenuList = std::vector < SubMenuListEntry >;
-WX_DEFINE_USER_EXPORTED_ARRAY(CommandListEntry *, CommandList, class AUDACITY_DLL_API);
+
+// This is an array of pointers, not structures, because the hash maps also point to them,
+// so we don't want the structures to relocate with vector operations.
+using CommandList = std::vector < std::unique_ptr<CommandListEntry> > ;
 
 WX_DECLARE_STRING_HASH_MAP_WITH_DECL(CommandListEntry *, CommandNameHash, class AUDACITY_DLL_API);
 WX_DECLARE_HASH_MAP_WITH_DECL(int, CommandListEntry *, wxIntegerHash, wxIntegerEqual, CommandIDHash, class AUDACITY_DLL_API);
@@ -97,6 +100,9 @@ class AUDACITY_DLL_API CommandManager: public XMLTagHandler
    CommandManager();
    virtual ~CommandManager();
 
+   CommandManager(const CommandManager&) = delete;
+   CommandManager &operator= (const CommandManager&) = delete;
+
    void PurgeData();
 
    //