c++ - resueltos - Consejo de refabricación: cómo evitar la verificación de tipos en este diseño OO

miembros de una clase en programacion orientada a objetos

Question

Estoy buscando consejos sobre la refactorización para mejorar el diseño de mi clase y evitar la verificación de tipos.

Estoy usando el patrón de diseño Command para construir un árbol de menú. Un elemento en el menú podría ser de varios tipos (por ejemplo, una acción inmediata [como "Guardar"], una propiedad activar / desactivar alternar que se muestra con el ícono / verificación dependiendo de su estado [como "cursivas"], etc.). Fundamentalmente, también hay submenús que reemplazan (en lugar de mostrarse al costado de) el menú actual en la pantalla. Estos submenús, por supuesto, contienen su propia lista de elementos de menú, que podría tener más submenús anidados.

El código es algo así como (todo público por simplicidad en la presentación):

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

El menú principal es solo una instancia de Menú, y una clase separada realiza un seguimiento de la posición del menú, incluyendo cómo entrar y salir de los submenús:

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

La función clave es Position :: OnEnterPressed (), donde se ve una comprobación de tipo explícita en la llamada a MenuItem :: IsMenu () y luego un molde para el tipo derivado. ¿Cuáles son algunas opciones para refactorizar esto para evitar el control de tipo y el lanzamiento?

Answer 1

El lenguaje ya proporciona este mecanismo: es dynamic_cast . Sin embargo, en un sentido más general, el defecto inherente en su diseño es este:

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

Debería ir en la función Execute () y refactorizar según sea necesario para que eso suceda.

Answer 2

IMO, el punto de partida de refactorización sería estas declaraciones:

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

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

El hecho de que el mismo tipo de afirmación se repita es IMO, el signo de la necesidad de refactorizar eso.

Si pudieras factorizar (1) en un método de tu clase base, y luego anularlo en la clase derivada para tener en cuenta el comportamiento específico (2), entonces podrías simplemente poner esto en Execute .

Corrígeme si estoy equivocado: la idea es que un menú tiene elementos, y cada elemento tiene una acción asociada que se activa cuando se detecta algún evento.

Ahora, cuando el elemento que selecciona es un submenú, la acción Execute tiene el significado: activar el submenú (estoy usando activar en un sentido genérico). Cuando el elemento no es un submenú, entonces Execute es una bestia diferente.

No tengo una comprensión completa de su sistema de menú, pero me parece que tiene una especie de menú / submenú de jerarquía (las posiciones) y algunas acciones que se desencadenan dependiendo del tipo de nodo.

Lo que visualizo es que la relación menú / submenú es una jerarquía que le permite definir nodos hoja (cuando no tiene un submenú) y nodos hoja (el submenú). Un nodo hoja invoca una acción, un nodo no hoja invoca un tipo diferente de acción que trata de activar un submenú (esta acción vuelve al sistema de menú, por lo que no encapsula el conocimiento sobre el sistema de menú en él, simplemente transmitir la acción al sistema de menú).

No sé si esto tiene sentido para ti.

Answer 3

Probablemente esta no sea la respuesta que estaba buscando, pero en mi opinión, su solución es muy superior a cualquier solución que no implique verificación de tipo.

La mayoría de los programadores de C ++ se ofenden con la idea de que necesites verificar el tipo de objeto para decidir qué hacer con él. Sin embargo, en otros lenguajes como Objective-C y la mayoría de los lenguajes de script de tipo débil, esto es muy recomendable.

En su caso, creo que el uso de la verificación de tipo está bien elegido ya que necesita la información de tipo para la funcionalidad de Position . Mover esta funcionalidad a una de las subclases de MenuItem mi humilde opinión violaría la separación de competencias. Position se refiere a la parte de visualización y control de su menú. No veo por qué el Menu clases de modelo o MenuItem debería preocuparse por eso. Pasar a una solución de verificación sin tipo disminuiría la calidad del código en términos de orientación del objeto.

Answer 4

Una alternativa sería exponer un método en Posición que permita que un Menú sea empujado a la pila, y llamar a ese método al inicio del Menú: Ejecutar. Entonces, el cuerpo de OnEnterPressed simplemente se convierte

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

Answer 5

Lo que necesita es la capacidad de expresar "ya sea una acción o un menú", que es muy engorroso escribir usando polimorfismo si las acciones y los menús tienen interfaces muy diferentes.

En lugar de tratar de forzarlos a entrar en una interfaz común ( Execute es un nombre deficiente para el método del submenú), iría más allá que usted y usaría dynamic_cast .

Además, dynamic_cast siempre es superior a un flag y static_cast . Las acciones no son necesarias para decirle al mundo que no son submenús.

Reescrito en el C ++ más idiomático, da el siguiente código. Utilizo std::list debido a sus métodos de conveniencia splice , insert y remove que no invalidan los iteradores (una de las pocas buenas razones para usar listas enlazadas). También uso std::stack para hacer un seguimiento de los menús abiertos.

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

Traté de mantener el código simple. No dude en preguntar si algo no está claro.

[Acerca de la invalidación del iterador al hacer el splice , vea esta pregunta (tl; dr: está bien empalmar y mantener los iteradores antiguos siempre que no use un compilador demasiado antiguo).]