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

Ответ 1

Ваша карта строк для команд, которые я считаю хорошими. Вы могли бы даже отбросить имя командной строки для конструктора (т.е. Не следует ли StartCommand знать, что его команда "START"?) Если бы вы могли это сделать, создание объектов вашей команды намного проще:

Class c = commandMap.get(cmdName);
if (c != null)
    return c.newInstance();
else
    throw new IllegalArgumentException(cmdName + " is not as valid command");

Другим вариантом является создание enum всех ваших команд со ссылками на классы (предположим, что все ваши объекты команд реализуют CommandInterface):

public enum Command
{
    START(StartCommand.class),
    END(EndCommand.class);

    private Class<? extends CommandInterface> mappedClass;
    private Command(Class<? extends CommandInterface> c) { mappedClass = c; }
    public CommandInterface getInstance()
    {
        return mappedClass.newInstance();
    }
}

так как toString перечисления является его именем, вы можете использовать EnumSet для поиска нужного объекта и получения класса изнутри.

Ответ 2

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

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) перечисления используется для нахождения правильного метода factory. Если factory не существует, он выдает IllegalArgumentException. Мы можем использовать это как сигнал для создания объекта InvalidCommand.

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

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

Ответ 3

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

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

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

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

Ответ 4

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

Ответ 5

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

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

Ответ 6

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

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

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

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

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

command.setCommandString(cmdName);

Но использование clone() не выглядит столь же изящным, как использование отражения: (

Ответ 7

Принятие подхода Convetion vs Configuration и использование отражения для сканирования доступных объектов Command и загрузка их на вашу карту было бы способом. Затем у вас есть возможность открыть новые команды без перекомпиляции factory.

Ответ 8

Другим подходом к динамическому поиску загружаемого класса было бы опустить явную карту и просто попытаться построить имя класса из командной строки. Атрибут заголовка и алгоритм concatenate могут включать "START" → "com.mypackage.commands.StartCommand" и просто использовать рефлексию, чтобы попытаться создать экземпляр. Если вы не можете найти класс, как-нибудь сработайте (InvalidCommand или Exception your own).

Затем вы добавляете команды, просто добавляя один объект и начинаете его использовать.

Ответ 9

Один вариант для каждого типа команды должен иметь свой собственный factory. Это дает вам два преимущества:

1) Ваш общий factory не будет вызывать новый. Поэтому каждый тип команды может в будущем возвращать объект другого класса в соответствии с аргументами, следующими за пробелом в строке.

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

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

Ответ 10

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

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

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

Ответ 11

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

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

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

Я думаю, что я применил некоторые "Замените переключатель с полиморфизмом" -Refactoring здесь, если кто-нибудь задается вопросом об этом.

Ответ 12

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

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

Я сделал что-то подобное раньше, используя maven и APT.

Ответ 13

То, как я это делаю, - не иметь общий метод Factory.

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

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

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

И так далее.

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

Ответ 14

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

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

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

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

 import xx.example.Command.*;

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

Ответ 15

Иди со своей кишкой и подумай. Однако в этом решении ваш интерфейс Command теперь считается доступным для метода setCommandString (String 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);
     }
}

Ответ 16

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

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

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

Ответ 17

По крайней мере, ваша команда должна иметь getCommandString() - где StartCommand переопределяет, чтобы вернуть "START". Затем вы можете просто зарегистрировать или открыть классы.

Ответ 18

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

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