这段代码非常不令人满意:

/*
Given a command string in which the first 8 characters are the command name
padded on the right with whitespace, construct the appropriate kind of 
Command object.
*/
public class CommandFactory {
     public Command getCommand(String cmd) {
         cmdName = cmd.subString(0,8).trim();

         if(cmdName.equals("START")) {
             return new StartCommand(cmd);
         }
         if(cmdName.equals("END")) {
             return new EndCommand(cmd);
         }
         // ... more commands in more if blocks here
         // else it's a bad command.
         return new InvalidCommand(cmd);
     }
}

我对多个出口点没有悔改 - 结构很清楚。但我对一系列几乎相同的if语句并不满意。我考虑过将字符串映射到命令:

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

...然后使用Reflection从Map中查找相应类的实例。然而,虽然在概念上优雅,但这涉及相当多的反射代码,任何继承此代码的人都可能不会欣赏 - 尽管这些成本可能会被优势所抵消。将命令值硬编码到commandMap中的所有行几乎和if块一样糟糕。

如果工厂的构造函数可以扫描类的路径以查找Command的子类,查询它们的String表示,并自动将它们添加到其库中,那就更好了。

那么 - 我该如何重构呢?

我想有些框架可以免费提供给我这样的东西。让我们假设我无法将这些东西迁移到这样的框架中。

有帮助吗?

解决方案

我认为你的命令字符串映射是好的。您甚至可以将字符串命令名称分解为构造函数(即,不应该让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 来找到正确的对象并从内部获取类。

其他提示

以下代码如何:

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) public,如果您也可以使这种方式构建一个Command对象,则编译时间可用于其余代码。然后,您可以使用 CommandFactory.START.create(String cmd )来创建Command对象。

最后一个好处是您可以在Javadoc文档中轻松创建所有可用命令的列表。

除外
cmd.subString(0,8).trim();
部分来说,这对我来说并不是太糟糕。您可以使用Map并使用反射,但是,根据您添加/更改命令的频率,这可能不会给您带来太大的影响。

您应该记录为什么您只需要前8个字符,或者可能更改协议,以便更容易找出该字符串的哪个部分是命令(例如,在...之后放置一个标记,如':'或';'命令关键字)。

它不能直接回答你的问题,但为什么不抛出InvalidCommandException(或类似的东西),而不是返回InvalidCommand类型的对象?

除非有理由不能,否则我总是试图让我的命令实现无状态。如果是这种情况,您可以在命令接口中添加方法布尔标识符(String id)方法,该方法将告知此实例是否可用于给定的字符串标识符。然后你的工厂看起来像这样(注意:我没有编译或测试这个):

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

我喜欢你的想法,但如果你想避免反思,你可以将实例添加到HashMap中:

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

每当您需要命令时,只需克隆它:

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

然后,设置命令字符串:

command.setCommandString(cmdName);

但是使用clone()并不像使用反射那样优雅:(

采用Convetion vs Configuration方法并使用反射扫描可用的Command对象并将它们加载到地图中将是最佳选择。然后,您可以在不重新编译工厂的情况下公开新命令。

动态查找要加载的类的另一种方法是省略显式映射,并尝试从命令字符串构建类名。标题情况和连接算法可以变为“开始”状态。 - &GT; “com.mypackage.commands.StartCommand”,只需使用反射来尝试实例化它。如果找不到类,则以某种方式失败(InvalidCommand实例或您自己的Exception)。

然后只需添加一个对象即可添加命令并开始使用它。

一种选择是每种命令类型都有自己的工厂。这为您提供了两个优势:

1)你的通用工厂不会打电话给新工厂。因此,每个命令类型将来可以根据字符串中空格填充后面的参数返回不同类的对象。

2)在HashMap方案中,对于每个命令类,可以避免映射到实现SpecialisedCommandFactory接口的对象,而不是映射到类本身。实际上这个对象可能是单例,但不需要这样指定。您的通用getCommand然后调用专门的getCommand。

也就是说,工厂扩散可能会失控,而你拥有的代码是最简单的工作。就个人而言,我可能会保留原样:您可以比较源和规范中的命令列表,而不需要像以前称为CommandFactory.registerCommand的非本地注意事项,或者通过反射发现了哪些类。这并不令人困惑。对于不到一千个命令,它不太可能变慢。唯一的问题是您无法在不修改工厂的情况下添加新的命令类型。但是你所做的修改是简单而重复的,如果你忘了制作它,你会得到一个包含新类型的命令行的明显错误,所以它并不繁琐。

将这些重复的对象创建代码隐藏在工厂中并不是那么糟糕。如果它必须在某个地方完成,至少它就在这里,所以我不会太担心它。

如果确实希望对此做些什么,可以选择Map,但是从属性文件配置它,然后从该props文件构建地图。

不使用类路径发现路径(我不知道),您将始终修改2个地方:编写一个类,然后在某处添加映射(工厂,映射init或属性文件)。 / p>

考虑到这一点,您可以创建一些实例化类,例如:

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

当然,如果小小的课程不能说“是的,那就是开始,给我那个”,这会增加一大堆。或者“nope,不喜欢那样”,但是,你现在可以重新修改工厂以包含那些CommandCreators的列表,并且只询问每一个:“你喜欢这个命令吗?”并返回第一个接受CommandCreator的create_instance的结果。当然,现在看起来有点难以提取CommandCreator之外的前8个字符,所以我会重做,所以你将整个命令字符串传递给CommandCreator。

我认为我在这里应用了一些“用多态性替换开关”-Refactoring,以防万一有人对此感到奇怪。

我会通过反思去寻找地图和创作。如果扫描类路径太慢,您始终可以向类中添加自定义注释,在编译时运行注释处理器并将所有类名存储在jar元数据中。

然后,你唯一能做的就是忘记注释。

我前一段时间做了类似的事情,使用了maven和 APT

我这样做的方法是没有通用的Factory方法。

我喜欢使用Domain Objects作为我的命令对象。因为我使用Spring MVC,所以这是一个很好的方法,因为 DataBinder.setAllowedFields方法为我提供了很大的灵活性,可以将单个域对象用于多种不同的形式。

要获取命令对象,我在Domain对象类上有一个静态工厂方法。例如,在成员类中,我有像 -

这样的方法
public static Member getCommandObjectForRegistration();
public static Member getCommandObjectForChangePassword();

等等。

我不确定这是一个很好的方法,我从来没有看到它在任何地方建议而且只是想出了我自己的b / c我喜欢把这样的东西保存在一个地方的想法。如果有人认为有任何理由反对,请在评论中告诉我......

我建议尽可能避免反思。这有点邪恶。

您可以使用三元运算符使代码更简洁:

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

你可以介绍一个枚举。使每个枚举恒定为工厂是冗长的,并且还具有一些运行时内存成本。但是你可以每天查找枚举,然后使用==或切换。

 import xx.example.Command.*;

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

与你的直觉一起去反思。但是,在此解决方案中,现在假定您的Command接口可以访问setCommandString(String s)方法,因此newInstance很容易使用。此外,commandMap是任何带有String键(cmd)的映射到它们对应的Command类实例。

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);
     }
}
嗯,浏览,只是偶然发现了这个。我还能发表评论吗?

恕我直言,原来的if / else块码有没有错误。这很简单,简单必须始终是我们在设计中的第一次调用( http://c2.com/cgi / wiki?DoTheSimplestThingThatCouldPossiblyWork

这似乎是正确的,因为所提供的所有解决方案都比原始代码更少自我记录......我的意思是我们不应该编写我们的阅读代码而不是翻译...

至少,你的命令应该有一个getCommandString() - 其中StartCommand覆盖返回“START”。然后你可以注册或发现课程。

+1反思建议,它将为您提供更健全的课程结构。

实际上你可以做以下事情(如果你还没有考虑过) 创建与您期望的String相对应的方法作为getCommand()工厂方法的参数,然后您需要做的就是反射并调用()这些方法并返回正确的对象。

许可以下: CC-BY-SA归因
不隶属于 StackOverflow
scroll top