Рефакторинг метода фабрики Java

+1 на отражательном предложении, это даст Вам более нормальную структуру в Вашем классе.

На самом деле Вы могли сделать следующее (если бы Вы уже не думали об этом), создают методы, соответствующие Строке, которую Вы ожидали бы как аргумент Вашему getCommand () метод фабрики, тогда все, что необходимо сделать, отражение, и вызовите () эти методы и возвратите правильный объект.

По крайней мере Ваша команда должна иметь getCommandString () - где переопределения StartCommand для возврата "ЗАПУСКАЮТСЯ". Тогда можно просто зарегистрировать или обнаружить классы.

Пойдите с пищеварительным трактом и отразитесь. Однако в этом решении, Ваш интерфейс Command, как теперь предполагается, имеет setCommandString (Представьте s в виде строки), доступный метод, так, чтобы newInstance был легко применим. Кроме того, commandMap является любой картой со Строковыми ключами (cmd) для Управления экземплярами класса, которым они соответствуют.

public class CommandFactory {
     public Command getCommand(String cmd) {
        if(cmd == null) {
            return new InvalidCommand(cmd);
        }

        Class commandClass = (Class) commandMap.get(cmd);

        if(commandClass == null) {
            return new InvalidCommand(cmd);
        }

        try {
            Command newCommand = (Command) commandClass.newInstance();
            newCommand.setCommandString(cmd);
            return newCommand;
        }
        catch(Exception e) {
            return new InvalidCommand(cmd);
     }
}

Я предложил бы избежать отражения если вообще возможный. Это является несколько злым.

можно сделать код более кратким при помощи тернарного оператора:

 return 
     cmdName.equals("START") ? new StartCommand  (cmd) :
     cmdName.equals("END"  ) ? new EndCommand    (cmd) :
                               new InvalidCommand(cmd);

Вы могли представить перечисление. Создание каждой перечислимой константы, фабрика является подробной и также имеет некоторую стоимость памяти во время выполнения. Но Вы можете eaily поиск перечисление и затем использовать это с == или переключатель.

 import xx.example.Command.*;

 Command command = Command.valueOf(commandStr);
 return 
     command == START ? new StartCommand  (commandLine) :
     command == END   ? new EndCommand    (commandLine) :
                        new InvalidCommand(commandLine);

Путем я делаю это не должно иметь универсального Метода фабрики.

мне нравится использовать Объекты области в качестве моих объектов команды. Так как я использую Spring MVC, это - большой подход начиная с , метод DataBinder.setAllowedFields позволяет мне большую гибкость для использования единственного объекта области для нескольких различных форм.

Для получения объекта команды у меня есть статический метод фабрики для класса Объекта области. Например, в членском классе у меня были бы методы как -

public static Member getCommandObjectForRegistration();
public static Member getCommandObjectForChangePassword();

И так далее.

я не уверен, что это - большой подход, я никогда не видел, что он предложил где угодно и отчасти просто придумал его самостоятельно b/c, мне нравится идея сохранить вещи как это в одном месте. Если кто-либо видит какие-либо основания для возражения сообщенный мне в комментариях...

Я пошел бы для карты и создания через отражение. Если сканирование пути к классу является слишком медленным, можно всегда добавлять пользовательскую аннотацию к классу, иметь процессор аннотации, работающий во время компиляции, и хранить все имена классов в метаданных банки.

Затем единственная ошибка, которую можно сделать, забывает аннотацию.

я сделал что-то вроде этого только что, с помощью знатока и APT.

Думая об этом, Вы могли создать небольшие классы инстанцирования, как:

class CreateStartCommands implements CommandCreator {
    public bool is_fitting_commandstring(String identifier) {
        return identifier == "START"
    }
    public Startcommand create_instance(cmd) {
        return StartCommand(cmd);
    }
}

, Конечно, это добавляет целый набор, если крошечные классы, которые не могут сделать намного больше, чем, говорят "да, это - запуск, дайте мне, что" или "нет, не любите это", однако, можно теперь переделать фабрику, чтобы содержать список тех CommandCreators и просто спросить каждый из него: "Вам нравится эта команда?" и возвратите результат create_instance первого принятия CommandCreator. Конечно, это теперь смотрит вид akward для извлечения первых 8 символов за пределами CommandCreator, таким образом, я переделал бы это так, Вы передаете всю командную строку в CommandCreator.

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

Наличие этого повторяющегося создания объекта кодирует, все скрытые на фабрике не так плохи. Если бы это должно быть сделано где-нибудь, по крайней мере, это - все здесь, таким образом, я не волновался бы об этом слишком много.

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

, не идя путем открытия пути к классу (о котором я не знаю), Вы будете всегда изменять 2 места: запись класса и затем добавление отображения где-нибудь (фабрика, init карты или файл свойств).

Одна опция состояла бы в том, чтобы каждый тип команды имел свою собственную фабрику. Это дает Вам два преимущества:

1) Ваша универсальная фабрика не назвала бы новым. Таким образом, каждый тип команды мог в будущем возвращать объект различного класса согласно аргументам после дополнения пространства в строке.

2) В Вашей схеме HashMap, Вы могли избежать отражения, для каждого класса команды, отобразившись на объект, реализовав интерфейс SpecialisedCommandFactory, вместо того, чтобы отобразиться на сам класс. Этот объект на практике, вероятно, был бы одиночным элементом, но не должен быть определен как таковой. Ваш универсальный getCommand тогда называет специализированный getCommand.

Тем не менее быстрое увеличение фабрики может выйти из-под контроля, и код, который Вы имеете, является самой простой вещью, которая делает задание. Лично я, вероятно, оставил бы его, как это: можно сравнить списки команд в источнике и спецификации без нелокальных соображений как то, что, возможно, ранее назвало CommandFactory.registerCommand, или какие классы были обнаружены посредством отражения. Это не сбивает с толку. Это очень вряд ли будет медленно меньше чем для тысячи команд. Единственная проблема состоит в том, что Вы не можете добавить новые типы команды, не изменяя фабрику. Но модификация, которую Вы сделали бы, является простой и повторяющейся, и если Вы забываете делать ее, Вы получаете очевидную ошибку для командных строк, содержащих новый тип, таким образом, это не обременительно.

Другой подход к динамичному нахождению, что класс загружается, должен был бы опустить явную карту и просто попытаться создать имя класса из командной строки. Случай заголовка и конкатенирует, алгоритм мог повернуться, "ЗАПУСКАЮТСЯ"-> "com.mypackage.commands. StartCommand", и просто используют отражение, чтобы попытаться инстанцировать его. Сбой так или иначе (экземпляр InvalidCommand или собственное Исключение), если Вы не можете найти класс.

Тогда Вы добавляете команды только путем добавления одного объекта и начинаете использовать его.

Взятие Convetion по сравнению с подходом Конфигурации и использованием отражения для сканирования для доступных Объектов команды и загрузки их в карту было бы способом пойти. У Вас тогда есть способность представить новые Команды без того, чтобы перекомпилировать фабрики.

Мне нравится Ваша идея, но если Вы хотите избежать отражения, Вы могли бы добавить вместо этого экземпляры к HashMap:

commandMap = new HashMap();
commandMap.put("START",new StartCommand());

Каждый раз, когда Вам нужна команда, Вы просто клонируете ее:

command = ((Command) commandMap.get(cmdName)).clone();

И впоследствии, Вы устанавливаете командную строку:

command.setCommandString(cmdName);

, Но использующий клон () не звучит столь же изящным как использование отражения: (

Если нет причина, они не могут быть, я всегда пытаюсь сделать свои реализации команды не сохраняющими состояние. Если это так, можно добавить идентификатор булевской переменной метода (Идентификатор строки) метод к интерфейсу команды, который сказал бы, мог ли этот экземпляр использоваться для данного идентификатора строки. Тогда Ваша фабрика могла выглядеть примерно так (примечание: Я не скомпилировал или протестировал это):

public class CommandFactory {
    private static List<Command> commands = new ArrayList<Command>();       

    public static void registerCommand(Command cmd) {
        commands.add(cmd);
    }

    public Command getCommand(String cmd) {
        for(Command instance : commands) {
            if(instance.identifier(cmd)) {
                return cmd;
            }
        }
        throw new CommandNotRegisteredException(cmd);
    }
}

Не непосредственно ответ на Ваш вопрос, но почему Вы не бросаете InvalidCommandException (или что-то подобное), скорее затем возвращая объект типа InvalidCommand?

За исключением

cmd.subString(0,8).trim();

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

необходимо, вероятно, зарегистрировать, почему Вы только хотите первые 8 символов или возможно изменяете протокол, таким образом, легче выяснить, какая часть той строки является командой (например, поместите маркер как ':' или''; после ключевого слова команды).

Как насчет следующего кода:

public enum CommandFactory {
    START {
        @Override
        Command create(String cmd) {
            return new StartCommand(cmd);
        }
    },
    END {
        @Override
        Command create(String cmd) {
            return new EndCommand(cmd);
        }
    };

    abstract Command create(String cmd);

    public static Command getCommand(String cmd) {
        String cmdName = cmd.substring(0, 8).trim();

        CommandFactory factory;
        try {
            factory = valueOf(cmdName);
        }
        catch (IllegalArgumentException e) {
            return new InvalidCommand(cmd);
        }
        return factory.create(cmd);
    }
}

valueOf(String) из перечисления используется для нахождения корректного метода фабрики. Если фабрика не будет существовать, то она бросит IllegalArgumentException. Мы можем использовать это в качестве сигнала создать эти InvalidCommand объект.

дополнительное преимущество то, что, если бы можно сделать метод create(String cmd) общественность, если Вы также пробились бы построения времени компиляции Объекта команды, проверенного доступный остальной части Вашего кода. Вы могли тогда использовать CommandFactory.START.create(String cmd) создать Объект команды.

последнее преимущество - то, что можно легко создать список всей доступной команды в документации Javadoc.

Хм, просматриваю, и только что наткнулся на это. Могу я еще прокомментировать?

ИМХО нет ничего неправильного в исходном коде блока if / else. Это просто, и простота всегда должна быть нашим первым делом при проектировании ( http://c2.com/cgi/wiki?DoTheSimplestThingThatCouldPossibleWork )

Это кажется особенно правдой, поскольку все предлагаемые решения намного меньше самодокументируется, чем исходный код ... Я имею в виду, что мы не должны писать наш код для чтения, а не для перевода ...