3

我正在寻找有关重构的建议，以改进我的类设计并避免类型检查。

我正在使用命令设计模式来构建菜单树。菜单中的项目可以是各种类型（例如，立即操作[如“保存”]、根据其状态显示带有检查/图标的切换开/关属性[如“斜体”]等）。至关重要的是，还有子菜单，它们替换屏幕上的当前菜单（而不是显示在旁边）。这些子菜单当然包含它们自己的菜单项列表，其中可能有更多嵌套的子菜单。

代码类似于（为简单起见，全部公开）：

// 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;
};

主菜单只是 Menu 的一个实例，一个单独的类跟踪菜单位置，包括如何进入和退出子菜单：

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;
};

关键函数是 Position::OnEnterPressed()，您可以在其中看到对 MenuItem::IsMenu() 的调用中的显式类型检查，然后转换为派生类型。有哪些选项可以重构它以避免类型检查和强制转换？

4

5 回答 5

4

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() ) );

同一类声明重复本身的事实是，IMO，需要重构的标志。

如果您可以在基类的方法中考虑 (1)，然后在派生类中重写它以考虑特定行为 (2)，那么您可以将其放入Execute.

如果我错了，请纠正我：这个想法是菜单有项目，并且每个项目都有一个与之关联的操作，当检测到某些事件时会触发该操作。

现在，当您选择的项目是子菜单时，该Execute操作具有以下含义：激活子菜单（我使用的是一般意义上的激活）。当该项目不是子菜单时，Execute则为不同的野兽。

我对您的菜单系统没有完全理解，但在我看来，您有一种层次结构菜单/子菜单（位置），以及根据节点类型触发的一些操作。

我的设想是菜单/子菜单关系是一个层次结构，允许您定义叶节点（当您没有子菜单时）和非叶节点（子菜单）。叶节点调用一个动作，一个非叶节点调用另一种处理激活子菜单的动作（这个动作回到菜单系统，所以你不需要在其中封装关于菜单系统的知识，你只需将动作传递给菜单系统）。

不知道这对你是否有意义。

于 2011-08-04T16:47:40.770 回答
2

另一种方法是在 Position 中公开一个方法，该方法可以将 Menu 推入堆栈，并在 Menu:Execute 的开头调用该方法。然后 OnEnterPressed 的主体就变成了

(*m_pos.back().iter)->Execute();
于 2011-08-04T16:43:22.623 回答
1

这可能不是您正在寻找的响应，但在我看来，您的解决方案远远优于任何不涉及类型检查的解决方案。

大多数 C++ 程序员对您需要检查对象的类型以决定如何处理它的想法感到不快。然而，在其他语言（如 Objective-C）和大多数弱类型脚本语言中，这实际上是受到高度鼓励的。

在你的情况下，我认为使用类型检查是很好的选择，因为你需要类型信息来实现Position. 将此功能移至MenuItem子类之一将恕我直言违反权限分离。Position与菜单的查看和控制器部分有关。我不明白为什么模型类Menu或MenuItem应该关注这一点。转向无类型检查解决方案会降低面向对象的代码质量。

于 2011-08-04T16:47:21.387 回答
1

您需要的是表达“动作或菜单”的能力，如果动作和菜单具有非常不同的接口，使用多态来编写非常麻烦。

与其试图强迫它们进入一个通用界面（Execute对于 submenu 方法来说这是一个糟糕的名字），我会比你走得更远，使用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，请参阅这个问题（tl;dr：如果您不使用太旧的编译器，可以拼接并保留旧的迭代器）。]

于 2011-08-04T17:33:50.757 回答
0

该语言已经提供了这种机制——它是dynamic_cast. 但是，在更一般的意义上，您的设计中的固有缺陷是：

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

它应该进入 Execute() 函数，并根据需要进行重构以实现这一点。

于 2011-08-04T16:46:04.503 回答