Java 팩토리 메소드 리팩토링

Question

이 코드에는 매우 만족스럽지 못한 점이 있습니다.

/*
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을 사용하여 지도에서 검색된 적절한 클래스의 인스턴스를 만듭니다.그러나 개념적으로는 우아하지만 이 코드를 상속받은 사람이 인식하지 못할 상당한 양의 Reflection 코드가 포함됩니다. 물론 그 비용이 이점으로 상쇄될 수도 있습니다.commandMap에 값을 하드코딩하는 모든 행은 if 블록만큼 나쁜 냄새를 풍깁니다.

팩토리의 생성자가 클래스 경로에서 Command의 하위 클래스를 검색하고 문자열 표현을 쿼리한 다음 자동으로 레퍼토리에 추가할 수 있다면 더욱 좋습니다.

그렇다면 어떻게 리팩토링해야 할까요?

아마도 일부 프레임워크에서는 이런 종류의 기능을 무료로 제공하는 것 같습니다.내가 이 내용을 그러한 프레임워크로 마이그레이션할 수 있는 위치에 있지 않다고 가정해 보겠습니다.

Answer 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 올바른 개체를 찾고 내부에서 클래스를 가져옵니다.

Answer 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) 열거형은 올바른 팩토리 메소드를 찾는 데 사용됩니다.공장이 존재하지 않으면 다음이 발생합니다. IllegalArgumentException.우리는 이것을 신호로 사용하여 InvalidCommand 물체.

추가적인 이점은 이 방법을 만들 수 있다면 create(String cmd) 이 방식으로 Command 개체 컴파일 시간을 확인하여 나머지 코드에서도 사용할 수 있도록 하려면 public을 선택하세요.그런 다음 사용할 수 있습니다 CommandFactory.START.create(String cmd) Command 객체를 생성합니다.

마지막 이점은 Javadoc 문서에서 사용 가능한 모든 명령 목록을 쉽게 만들 수 있다는 것입니다.

Answer 3

다음을 제외하고는

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

제가 보기에는 별로 나쁘지 않은 것 같습니다.맵을 사용하여 리플렉션을 사용할 수 있지만 명령을 얼마나 자주 추가/변경하는지에 따라 별로 구매하지 못할 수도 있습니다.

처음 8자만 원하는 이유를 문서화하거나 프로토콜을 변경하여 해당 문자열의 어느 부분이 명령인지 더 쉽게 파악할 수 있습니다(예:':'또는 ';'와 같은 마커를 넣으십시오. 명령 키 워드 후).

Answer 4

귀하의 질문에 대한 직접적인 답변은 아니지만 InvalidCommand 유형의 객체를 반환하는 대신 InvalidCommandException(또는 이와 유사한 것)을 발생시키는 것이 어떻습니까?

Answer 5

그럴 수 없는 이유가 없는 한 저는 항상 명령 구현을 상태 비저장으로 만들려고 노력합니다.그런 경우에는 이 인스턴스가 주어진 문자열 식별자에 사용될 수 있는지 여부를 알려주는 부울 식별자(문자열 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);
    }
}

Answer 6

나는 당신의 아이디어가 마음에 들지만 반영을 피하고 싶다면 대신 HashMap에 인스턴스를 추가할 수 있습니다.

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

명령이 필요할 때마다 이를 복제하면 됩니다.

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

그런 다음 명령 문자열을 설정합니다.

command.setCommandString(cmdName);

하지만 clone()을 사용하는 것은 리플렉션을 사용하는 것만큼 우아하게 들리지 않습니다.

Answer 7

Convetion 대 ​​구성 접근 방식을 취하고 리플렉션을 사용하여 사용 가능한 Command 개체를 검색하고 이를 지도에 로드하는 것이 좋은 방법입니다.그러면 팩토리를 다시 컴파일하지 않고도 새 명령을 노출할 수 있습니다.

Answer 8

로드할 클래스를 동적으로 찾는 또 다른 접근 방식은 명시적인 맵을 생략하고 명령 문자열에서 클래스 이름을 빌드하는 것입니다.제목 케이스와 연결 알고리즘은 "START" -> "com.mypackage.commands.StartCommand"로 바뀔 수 있으며 리플렉션을 사용하여 인스턴스화를 시도합니다.클래스를 찾을 수 없으면 어떻게든 실패합니다(InvalidCommand 인스턴스 또는 사용자 고유의 예외).

그런 다음 개체 하나를 추가하여 명령을 추가하고 사용을 시작합니다.

Answer 9

한 가지 옵션은 각 명령 유형이 자체 팩토리를 갖는 것입니다.이는 두 가지 이점을 제공합니다.

1) 일반 공장에서는 새 제품을 부르지 않습니다.따라서 각 명령 유형은 나중에 문자열에서 공백 채우기 뒤에 오는 인수에 따라 다른 클래스의 객체를 반환할 수 있습니다.

2) HashMap 구성표에서는 각 명령 클래스에 대해 클래스 자체에 매핑하는 대신 SpecialisedCommandFactory 인터페이스를 구현하는 개체에 매핑하여 반영을 피할 수 있습니다.실제로 이 개체는 싱글톤일 수 있지만 그렇게 지정할 필요는 없습니다.그러면 일반 getCommand가 특수한 getCommand를 호출합니다.

그렇긴 하지만, 공장 확장은 감당하기 어려울 수 있으며, 가지고 있는 코드는 작업을 수행하는 가장 간단한 것입니다.개인적으로 나는 아마도 이대로 놔둘 것입니다:이전에 CommandFactory.registerCommand를 호출했거나 리플렉션을 통해 발견된 클래스와 같은 비로컬 고려 사항 없이 소스 및 사양의 명령 목록을 비교할 수 있습니다.혼란스럽지 않습니다.천 개 미만의 명령에 대해서는 속도가 느려질 가능성이 거의 없습니다.유일한 문제는 팩토리를 수정하지 않고는 새로운 명령 유형을 추가할 수 없다는 것입니다.그러나 수정하는 작업은 간단하고 반복적이며 수정하는 것을 잊어버리면 새 유형이 포함된 명령줄에 대해 명백한 오류가 발생하므로 번거롭지 않습니다.

Answer 10

이러한 반복적인 객체 생성 코드를 공장에 모두 숨겨 두는 것은 그리 나쁘지 않습니다.어딘가에서 해야 할 일이라면 적어도 여기에는 다 있으니 크게 걱정하지 않아도 된다.

만약 너라면 정말 이에 대해 뭔가를 하고 싶을 수도 있습니다. 지도로 가서 속성 파일에서 구성하고 해당 props 파일에서 지도를 빌드할 수도 있습니다.

클래스 경로 검색 경로를 사용하지 않고(이에 대해서는 잘 모르겠습니다) 항상 두 곳을 수정하게 됩니다.클래스를 작성한 다음 어딘가(팩토리, 맵 초기화 또는 속성 파일)에 매핑을 추가합니다.

Answer 11

이에 대해 생각하면 다음과 같은 작은 인스턴스화 클래스를 만들 수 있습니다.

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

물론 이것은 "예, 시작합니다. 저에게 주세요" 또는 "아니요, 그건 마음에 들지 않습니다"라고 말하는 것 이상을 수행할 수 없는 작은 클래스의 경우 전체 무리를 추가합니다. 그러나 이제 팩토리를 재작업하여 다음과 같이 할 수 있습니다. 해당 CommandCreator 목록을 포함하고 각각에 대해 물어보세요."이 명령이 마음에 드십니까?" 첫 번째 수락 명령 크리징 자의 Create_instance의 결과를 반환하십시오.물론 이제 CommandCreator 외부에서 처음 8자를 추출하는 것이 다소 어색해 보이므로 전체 명령 문자열을 CommandCreator에 전달하도록 다시 작업하겠습니다.

누군가가 그것에 대해 궁금해할 경우를 대비해 "스위치를 다형성으로 교체" 리팩토링을 적용한 것 같습니다.

Answer 12

나는 반사를 통해지도와 생성을 위해 갈 것입니다.클래스 경로 스캔이 너무 느린 경우 언제든지 클래스에 사용자 정의 주석을 추가하고, 컴파일 타임에 주석 프로세서를 실행하고, 모든 클래스 이름을 jar 메타데이터에 저장할 수 있습니다.

그러면 당신이 할 수 있는 유일한 실수는 주석을 잊어버리는 것입니다.

나는 얼마 전에 maven을 사용하여 이와 같은 작업을 수행했습니다. 적절한.

Answer 13

내가 하는 방식은 일반적인 Factory 메서드를 사용하지 않는 것입니다.

저는 도메인 개체를 명령 개체로 사용하는 것을 좋아합니다.나는 Spring MVC를 사용하기 때문에 이것은 훌륭한 접근 방식이다. DataBinder.setAllowedFields 메서드 여러 가지 다른 형태에 대해 단일 도메인 개체를 사용할 수 있는 유연성이 뛰어납니다.

명령 개체를 얻으려면 도메인 개체 클래스에 정적 팩터리 메서드가 있습니다.예를 들어, 멤버 클래스에는 다음과 같은 메소드가 있습니다.

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

등등.

이것이 훌륭한 접근 방식인지는 잘 모르겠습니다. 어디에서도 제안된 것을 본 적이 없으며 그냥 제 스스로 생각해낸 것 같습니다. 저는 이와 같은 것을 한 곳에 보관하는 아이디어를 좋아합니다.반대할 이유가 있는 사람이 있으면 댓글로 알려주시기 바랍니다.

Answer 14

가능하다면 성찰을 피하는 것이 좋습니다.다소 사악합니다.

삼항 연산자를 사용하면 코드를 더 간결하게 만들 수 있습니다.

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

Answer 15

직감으로 가서 반성하십시오.그러나 이 솔루션에서는 이제 Command 인터페이스에 setCommandString(String s) 메서드에 액세스할 수 있는 것으로 가정하므로 newInstance를 쉽게 사용할 수 있습니다.또한 commandMap은 해당하는 Command 클래스 인스턴스에 대한 문자열 키(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);
     }
}

Answer 16

흠, 검색하다가 방금 이걸 발견했습니다.그래도 댓글을 달 수 있나요?

IMHO 거기 있어요 아무것도 아님 원래 if/else 블록 코드에 문제가 있습니다.이것은 간단하며, 디자인에서 단순성은 항상 우리의 첫 번째 요구 사항이어야 합니다(http://c2.com/cgi/wiki?DoTheSimplestThingThatCouldPossicallyWork)

제공되는 모든 솔루션이 원래 코드보다 자체 문서화 수준이 훨씬 낮기 때문에 이는 특히 사실인 것 같습니다. 번역보다는 읽기를 위해 코드를 작성하면 안 된다는 뜻입니다...

Answer 17

최소한 명령에는 getCommandString()이 있어야 합니다. 여기서 StartCommand는 "START"를 반환하도록 재정의됩니다.그런 다음 수업을 등록하거나 검색할 수 있습니다.

Answer 18

반영 제안에 +1하면 수업에서보다 건전한 구조를 제공할 것입니다.

실제로 다음을 수행 할 수 있습니다. )이 방법은 올바른 개체를 반환합니다.