Рекомендации по рефакторингу для больших коммутаторов в C # - PullRequest
Новая
       23

Рекомендации по рефакторингу для больших коммутаторов в C #

2 голосов
/ 03 июня 2009

У меня есть приложение на C # / Winforms, которое позволяет пользователям размещать объекты в сетке для создания уровней для игры. У него есть несколько инструментов для размещения плиток / источников света / дверей / объектов и т. Д. В настоящее время я просто использую enum для хранения выбранного в данный момент инструмента и имею инструкцию switch для запуска каждого кода инструментов. Поскольку я добавляю больше инструментов в приложение, оно начинает выглядеть как спагетти с большим количеством дублированного кода.

Вот урезанная версия функции мыши в моем классе редактора: 

    public void OnEditorViewMouseDown(Point mousePos)
    {
        // Check if the click is out of bounds.
        if (IsLocationOOB(mousePos)) return;

        if (CurrentTool == ToolType.GroundTile)
        {
            // Allow drags along whole tiles only.
            m_DragManager.DragType = DragManager.DragTypeEnum.Tile;
            m_DragManager.StartDrag(mousePos);
        }
        else if (CurrentTool == ToolType.WallTile)
        {
            // Allow drags along grid edges only.
            m_DragManager.DragType = DragManager.DragTypeEnum.Edge;
            m_DragManager.StartDrag(mousePos);
        }
        else if (CurrentTool == ToolType.PostTile)
        {
            // Allow drags along grid points only.
            m_DragManager.DragType = DragManager.DragTypeEnum.Point;
            m_DragManager.StartDrag(mousePos);
        }
        else if (CurrentTool == ToolType.AreaLight)
        {
            // Allow drags anywhere. ie. not snapped to the grid in some way.
            m_DragManager.DragType = DragManager.DragTypeEnum.FreeForm;
            m_DragManager.StartDrag(mousePos);
        }
        else if (CurrentTool == ToolType.PointLight)
        {
            m_CurrentWorld.AddLight(TranslateToWorldCoords(mousePos));
        }
        else if (CurrentTool == ToolType.PlaceEntity)
        {
            m_CurrentWorld.PlaceEntity(TranslateToWorldCoords(mousePos));
        }
    }

Переключатель используется в нескольких других функциях (OnMouseMove, OnMouseUp) и выглядит как плохой дизайн (большой переключатель скопирован в нескольких функциях). Любой совет для рефакторинга что-то вроде этого в более чистой и расширяемой форме? В настоящее время я думаю о базовом классе Tool и о том, что каждый инструмент имеет свой собственный класс, который переопределяет используемые им функции (OnMouseDown () и т. Д.). Это звучит разумно?

Спасибо за чтение.

Ответы [ 7 ]

6 голосов
/ 03 июня 2009

У вас хорошая интуиция.

Обычно в ООП, когда у вас есть строки переключателей if или огромных, это сильный запах кода. Лучший способ избавиться от этого запаха - это пойти с полиморфизмом.

Вы должны продолжить свою идею, имея базовый абстрактный класс BaseTool, с различными методами OnXXX, реализованными как nops (просто возвращает, поэтому вам нужно только указать поведение, если ваш инструмент знает, как действовать на метод), и каждый инструмент наследует от BaseTool и реализует свое собственное поведение, переопределяя соответствующие методы.

Итак, ваш метод в конечном итоге 

public void OnEditorViewMouseDown(Point mousePos)
{
  currentTool.OnEditorViewMouseDown(mousePos);
}

В зависимости от вашего проекта, вы также должны рассмотреть возможность передачи DragManager методу, чтобы не привязывать к переменным экземпляра, лежащим вокруг. EditorContext (содержащий DragManager), оснащенный всем необходимым для метода без необходимости извлечения «глобальных» переменных, сделает ваш метод более автономным и менее хрупким при рефакторинге. Сам дизайн будет зависеть от ответственности: кто за что отвечает.

5 голосов
/ 03 июня 2009

Звучит как хорошее место для использования паттерна стратегии: http://www.google.com/search?q=c%23+strategy+pattern

3 голосов
/ 03 июня 2009

Да, полиморфизм - это то, что вы хотите здесь. Вы должны определить либо abstract базовый класс Tool, либо интерфейс ITool в зависимости от того, хотите ли вы добавить реализацию к базе или нет (т. Е. Если есть общие функциональные возможности для всех инструментов, вы можете использовать абстрактный базовый класс ).

Ваш менеджер должен тогда взять инструмент или ITool, когда что-то нужно сделать. Ваш инструмент реализует функцию перетаскивания, которая получает необходимую информацию и либо возвращает то, что ему нужно вернуть, либо делает то, что нужно. Или вы можете реализовать инверсию элемента управления между вашим менеджером и вашим инструментом и добавить менеджер в инструмент через внедрение свойств (Tool.Manager = dragManager) и позволить инструменту делать то, что ему нужно, с помощью менеджера. .

3 голосов
/ 03 июня 2009

Да, вы должны иметь базовый класс (или, по крайней мере, интерфейс), который определяет все общие методы, необходимые для всех инструментов. Попробуйте заставить этот код работать, и он даст вам хорошее представление о том, как проектировать ваши классы: 

m_DragManager.DragType = CurrentTool.DragType;
m_DragManager.StartDrag(mousePos);

где "CurrentTool" - это экземпляр вашего базового класса или вашего интерфейса.

Таким образом, в основном, когда выбран «Инструмент», в этот момент вы определяете, с каким производным Инструментом вы имеете дело, но с этого момента вы имеете дело только с базовым классом и забываете о любых перечислениях или о чем-либо подобном чтобы определить текущий выбранный инструмент. Имеет смысл?

2 голосов
/ 03 июня 2009
1 голос
/ 03 июня 2009

Я не очень знаком с синтаксисом C #, но в целом вы могли бы сделать CurrentTool объектом, имеющим методы StartDrag, EndDrag, которые принимают DragManager в качестве аргумента. Затем, когда мышь нажимается вниз на виде, вы просто вызываете CurrentTool.StartDrag(m_DragManager).

0 голосов
/ 03 июня 2009

Попробуйте перечислить флаг. Каждый бит будет отличаться от других и позволит лучше составлять элементы на ячейку.

Тогда вы можете просто проверить каждый бит даже разными способами.

...