重構建議：如何避免在此OO設計中進行類型檢查

Question

我正在尋找重構的建議以改進我的類設計並避免類型檢查。

我正在使用Command設計模式來構建菜單樹。菜單中的項目可以是各種類型的（例如，立即動作[比如「保存」]，切換開/關屬性，其根據其狀態（例如「斜體」等）隨着檢查/圖標顯示。重要的是，還有子菜單，其中取代屏幕上當前菜單的（而不是顯示在側面）。這些子菜單當然包含它們自己的菜單項列表，這些菜單項可以有更多的嵌套子菜單。

的代碼是一樣的東西（所有公衆簡單的演示）：

// Abstract base class 
struct MenuItem 
{ 
    virtual ~MenuItem() {} 
    virtual void Execute()  = 0; 
    virtual bool IsMenu() const = 0; 
}; 

// Concrete classes 
struct Action : MenuItem 
{ 
    void Execute() { /*...*/ } 
    bool IsMenu() const { return false; } 
    // ... 
}; 

// ... other menu items 

struct Menu : MenuItem 
{ 
    void Execute() { /* Display menu */ } 
    bool IsMenu() const { return true; } 
    // ... 
    std::vector<MenuItem*> m_items; 
    typedef std::vector<MenuItem*>::iterator ItemIter; 
}; 

的主菜單就是菜單的一個實例，以及一個單獨的類保留菜單位置的軌跡，包括如何去進，出子菜單：

struct Position 
{ 
    Position(Menu* menu) 
    : m_menu(menu) 
    { 
    // Save initial position 
    m_pos.push_back(MenuPlusIter(m_menu, m_menu->m_items.begin())); 
    } 

    // Ignore error conditions for simplicity 
    void OnUpPressed() { m_pos.back().iter--; } 
    void OnDownPressed() { m_pos.back().iter++; } 
    void OnBackPressed() { m_pos.pop_back(); } 

    void OnEnterPressed() 
    { 
    MenuItem* item = *m_pos.back().iter; 
    // Need to behave differently here if the currently 
    // selected item is a submenu 
    if(item->IsMenu()) 
    { 
     // dynamic_cast not needed since we know the type 
     Menu* submenu = static_cast<Menu*>(item); 

     // Push new menu and position onto the stack 
     m_pos.push_back(MenuPlusIter(submenu, submenu->m_items.begin())); 

     // Redraw 
     submenu->Execute(); 
    } 
    else 
    { 
     item->Execute(); 
    } 
    } 

private: 
    struct MenuPlusIter 
    { 
     Menu*   menu; 
     Menu::ItemIter iter; 

     MenuPlusIter(Menu* menu_, Menu::ItemIter iter_) 
      : menu(menu_) 
      , iter(iter_) 
     {} 
    }; 

    Menu* m_menu; 
    std::vector<MenuPlusIter> m_pos; 
}; 

的主要功能是位置:: OnEnterPressed（），在那裏你看到在調用菜單項）一個明確的類型檢查:: IsMenu（然後強制轉換爲派生類型。有什麼方法可以重構這個以避免類型檢查和轉換？

Answer 1

IMO，重構的起點是這些語句：

1. m_pos.push_back(MenuPlusIter(m_menu, m_menu->m_items.begin())); 

2. m_pos.push_back(MenuPlusIter(submenu, submenu->m_items.begin())); 

，同樣的一種說法何其事實是，國際海事組織，爲需要重構的標誌。

如果你可以在你的基類的方法中考慮因素（1），然後在派生類中重寫它來考慮具體行爲（2），那麼你可以把它放在Execute。

糾正我，如果我錯了：這個想法是一個菜單有項目，每個項目都有一個相關的行動，當檢測到一些事件時觸發。

現在，當您選擇的項目是子菜單時，Execute操作具有以下含義：激活子菜單（我正在使用通用意義上的激活）。當物品不是子菜單時，Execute是不同的野獸。

我沒有對菜單系統的全面理解，但在我看來，您有一種層次結構菜單/子菜單（位置），以及根據節點種類觸發的一些操作。

我的設想是，菜單/子菜單關係是一個層次結構，允許您定義葉節點（如果沒有子菜單）和非葉節點（子菜單）。一個葉節點調用一個動作，一個非葉節點調用另一種處理激活子菜單的動作（這個動作返回到菜單系統，所以你不會在其中封裝有關菜單系統的知識，你只需將動作轉發給菜單系統）。

不知道這對你是否有意義。

Answer 2

另一種方法是公開Position中的一個方法，該方法可以將菜單壓入堆棧，並在Menu：Execute開始時調用該方法。然後OnEnterPressed的身體剛剛成爲

(*m_pos.back().iter)->Execute(); 

Answer 3

語言已經提供了這種機制 - 這是dynamic_cast。然而，在更一般的意義上，在你的設計中的固有缺陷是這樣的：

m_pos.push_back(MenuPlusIter(submenu, submenu->m_items.begin())); 

它應該在執行（）函數，並重構所必需的做到這一點。

Answer 4

這可能不是您正在尋找的響應，但在我看來，您的解決方案遠遠優於任何不涉及類型檢查的解決方案。

大多數C++程序員因爲需要檢查對象的類型以決定如何處理它而受到冒犯。然而在其他語言如Objective-C和大多數弱類型的腳本語言中，這實際上是非常受鼓勵的。

在你的情況，我認爲使用類型檢查是很好的選擇，因爲你需要類型信息的功能Position。將此功能移至MenuItem子類中的一個，恕我直言將違反能力分離。 Position與您的菜單的查看和控制器部分有關。我不明白爲什麼模型類Menu或MenuItem應該與此有關。轉向無類型檢查解決方案會降低代碼質量的對象方向。

Answer 5

您需要的是能夠表達「動作或菜單」，如果動作和菜單具有非常不同的界面，使用多態性編寫起來非常麻煩。

而不是試圖強制他們進入一個共同的界面（Execute是一個不好的名稱爲子菜單方法），我會走得比你更遠，並使用dynamic_cast。

另外，dynamic_cast是總是優於國旗和static_cast。行動不能告訴世界他們不是子菜單。

用最習慣的C++重寫了這個代碼。我使用std::list，因爲其方便的方法splice,insert和remove不會使迭代器無效（使用鏈接列表的幾個好理由之一）。我也使用std::stack來跟蹤打開的菜單。

struct menu_item 
{ 
    virtual ~menu_item() {} 

    virtual std::string label() = 0; 
    ... 
}; 

struct action : menu_item 
{ 
    virtual void execute() = 0; 
}; 

struct submenu : menu_item 
{ 
    // You should go virtual here, and get rid of the items member. 
    // This enables dynamically generated menus, and nothing prevents 
    // you from having a single static_submenu class which holds a 
    // vector or a list of menu_items. 
    virtual std::list<menu_item*> unfold() = 0; 
}; 

struct menu 
{ 
    void on_up() { if (current_item != items.begin()) current_item--; } 
    void on_down() { if (++current_item == items.end()) current_item--; } 

    void on_enter() 
    { 
     if (submenu* m = dynamic_cast<submenu*>(current_item)) 
     { 
      std::list<menu_item*> new_menu = m->unfold(); 

      submenu_stack.push(submenu_info(*current_item, new_menu)); 

      items.splice(current_item, new_menu); 
      items.erase(current_item); 
      current_item = submenu_stack.top().begin; 

      redraw(current_item, items.end()); 
     } 

     else if (action* a = dynamic_cast<action*>(current_item)) 
      a->execute(); 

     else throw std::logic_error("Unknown menu entry type"); 

     // If we were to add more else if (dynamic_cast) clauses, this 
     // would mean we don't have the right design. Here we are pretty 
     // sure this won't happen. This is what you say to coding standard 
     // nazis who loathe RTTI. 
    } 

    void on_back() 
    { 
     if (!submenu_stack.empty()) 
     { 
      const submenu_info& s = submenu_stack.top(); 

      current_item = items.insert(items.erase(s.begin, s.end), s.root); 
      submenu_stack.pop(); 

      redraw(current_item, items.end()); 
     } 
    } 

    void redraw(std::list<menu_item*>::iterator begin, 
       std::list<menu_item*>::iterator end) 
    { 
     ... 
    } 

private: 
    std::list<menu_item*> items; 
    std::list<menu_item*>::iterator current_item; 

    struct submenu_info 
    { 
     submenu* root; 
     std::list<menu_item*>::iterator begin, end; 

     submenu_info(submenu* root, std::list<menu_item*>& m) 
      : root(root), begin(m.begin()), end(m.end()) 
     {} 
    }; 

    std::stack<submenu_info> submenu_stack; 
}; 

我試着讓代碼直截了當。隨意詢問是否有不清楚的地方。

[關於迭代器失效做splice時，看到this question（TL;博士：它是確定拼接和保留舊的迭代器提供您不要使用太舊的編譯器）。]