프로그래밍은 과학보다 공예(craft)에 가깝다.
깨끗한 코드를 짜려면 먼저 지저분한 코드를 짠 뒤에 정리해야 한다.
책 초반에 잘 정돈된 코드가 먼저 나오는데, 처음부터 이 코드를 짠 것이 아니라 기능을 하나씩 덧붙여나가다가 코드가 점점 알아보기 힘들어지자 리팩토링을 시작했다는 점에 주목하면 좋을 것 같습니다.
초안
인수로 넘어오는 문자열 배열을 분석하는 클래스
Args arg = new Args("l,p#,d*", args); // args는 문자열 배열public class Args {
...
private Map<Character, Boolean> booleanArgs = new HashMap<Character, Boolean>();
private Map<Character, String> stringArgs = new HashMap<Character, String>();
private Map<Character, Integer> intArgs = new HashMap<Character, Integer>();
...
public Args(String schema, String[] args) throws ParseException {
this.schema = schema;
this.args = args;
valid = parse();
}
...
private void parseSchemaElement(String element) throws ParseException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (isBooleanSchemaElement(elementTail))
parseBooleanSchemaElement(elementId);
else if (isStringSchemaElement(elementTail))
parseStringSchemaElement(elementId);
else if (isIntegerSchemaElement(elementTail))
parseIntegerSchemaElement(elementId);
else
throw new ParseException(String.format("Argument: %c has invalid format: %s.",
elementId, elementTail), 0);
}
}
...
private void parseBooleanSchemaElement(char elementId) {
booleanArgs.put(elementId, false);
}
private void parseIntegerSchemaElement(char elementId) {
intArgs.put(elementId, 0);
}
private void parseStringSchemaElement(char elementId) {
stringArgs.put(elementId, "");
}
private boolean isStringSchemaElement(String elementTail) {
return elementTail.equals("*");
}
private boolean isBooleanSchemaElement(String elementTail) {
return elementTail.length() == 0;
}
private boolean isIntegerSchemaElement(String elementTail) {
return elementTail.equals("#");
}
...
private boolean setArgument(char argChar) throws ArgsException {
if (isBooleanArg(argChar))
setBooleanArg(argChar, true);
else if (isStringArg(argChar))
setStringArg(argChar);
else if (isIntArg(argChar))
setIntArg(argChar);
else
return false;
return true;
}
private boolean isIntArg(char argChar) {
return intArgs.containsKey(argChar);
}
private void setIntArg(char argChar) throws ArgsException {
currentArgument++;
String parameter = null;
try {
parameter = args[currentArgument];
intArgs.put(argChar, new Integer(parameter));
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_INTEGER;
throw new ArgsException();
} catch (NumberFormatException e) {
valid = false;
errorArgumentId = argChar;
errorParameter = parameter;
errorCode = ErrorCode.INVALID_INTEGER;
throw new ArgsException();
}
}
private void setStringArg(char argChar) throws ArgsException {
currentArgument++;
try {
stringArgs.put(argChar, args[currentArgument]);
} catch (ArrayIndexOutOfBoundsException e) {
valid = false;
errorArgumentId = argChar;
errorCode = ErrorCode.MISSING_STRING;
throw new ArgsException();
}
}
private boolean isStringArg(char argChar) {
return stringArgs.containsKey(argChar);
}
private void setBooleanArg(char argChar, boolean value) {
booleanArgs.put(argChar, value);
}
private boolean isBooleanArg(char argChar) {
return booleanArgs.containsKey(argChar);
}
...
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return unexpectedArgumentMessage();
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.", errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.", errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.", errorArgumentId);
}
return "";
}
...
public String getString(char arg) {
return blankIfNull(stringArgs.get(arg));
}
public int getInt(char arg) {
return zeroIfNull(intArgs.get(arg));
}
public boolean getBoolean(char arg) {
return falseIfNull(booleanArgs.get(arg));
}
...
}
예제의 코드를 모두 하나하나 읽기 보다는 전체적으로 반복되는 패턴에 집중함
Boolean, String, integer 와 같은 다양한 타입에 대응하지만,
잘 살펴보면 비슷한 형식(parse, set, get)을 가지고 있음을 확인할 수 있음
각 타입마다 인스턴스 변수와 메서드를 따로 가지고 있어서 인스턴스 변수와 메서드가 많아짐. 그렇기 때문에
각 메서드에서 사용하지 않는 인스턴스 변수가 많아 응집도가 떨어지고 결합도가 높은 클래스라고 볼 수 있음.
따라서 이를 개선하기 위한 가장 쉬운 방법은 연관된 개념을 가지는 인스턴스 변수와 메서드들을 객체로 분리하는 것. 이 경우에는 Boolean, String, integer 가 유사한 형식(set, get)을 가지므로 인터페이스를 만들고 해당 인터페이스를 구현하는 Boolean, String, integer 관련 객체를 만들 수 있다.
프로그램을 망치는 가장 좋은 방법 중 하나는 개선이라는 이름 아래 구조를 크게 뒤집는 행위다.
그렇기 때문에 TDD를 통해 테스트를 진행하며 점진적으로 개선한다.
private 메서드의 로직은 검증하지 않아도 되는지?
https://anthonysciamanna.com/2016/02/14/should-private-methods-be-tested.html
개선한 코드
ArgumentMarshaler
public interface ArgumentMarshaler {
void set(Iterator<String> currentArgument) throws ArgsException;
Object get();
}public class BooleanArgumentMarshaler implements ArgumentMarshaler {
private boolean booleanValue = false;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
booleanValue = Boolean.parseBoolean(parameter);
} catch (NoSuchElementException e) {
throw new ArgsException(MISSING_BOOLEAN);
} catch (NumberFormatException e) {
throw new ArgsException(INVALID_BOOLEAN, parameter);
}
}
public Object get() {
return booleanValue;
}
}public class IntegerArgumentMarshaler implements ArgumentMarshaler {
private int intValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
intValue = Integer.parseInt(parameter);
} catch (NoSuchElementException e) {
throw new ArgsException(MISSING_INTEGER);
} catch (NumberFormatException e) {
throw new ArgsException(INVALID_INTEGER, parameter);
}
}
public Object get() {
return intValue;
}
}public class DoubleArgumentMarshaler implements ArgumentMarshaler {
private double doubleValue = 0;
public void set(Iterator<String> currentArgument) throws ArgsException {
String parameter = null;
try {
parameter = currentArgument.next();
doubleValue = Double.parseDouble(parameter);
} catch (NoSuchElementException e) {
throw new ArgsException(MISSING_DOUBLE);
} catch (NumberFormatException e) {
throw new ArgsException(INVALID_DOUBLE, parameter);
}
}
public Object get() {
return doubleValue;
}
}ArgsException
예외처리와 관련된 인스턴스 변수와 메서드 또한 분리
public class ArgsException extends Exception {
private char errorArgumentId = '\0';
private String errorParameter = "TILT";
private ErrorCode errorCode = ErrorCode.OK;
public ArgsException() {}
public ArgsException(String message) {super(message);}
public ArgsException(ErrorCode errorCode) {
this.errorCode = errorCode;
}
public ArgsException(ErrorCode errorCode, String errorParameter) {
this.errorCode = errorCode;
this.errorParameter = errorParameter;
}
...
public String errorMessage() throws Exception {
switch (errorCode) {
case OK:
throw new Exception("TILT: Should not get here.");
case UNEXPECTED_ARGUMENT:
return String.format("Argument -%c unexpected.", errorArgumentId);
case MISSING_STRING:
return String.format("Could not find string parameter for -%c.",
errorArgumentId);
case INVALID_INTEGER:
return String.format("Argument -%c expects an integer but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_INTEGER:
return String.format("Could not find integer parameter for -%c.",
errorArgumentId);
case INVALID_DOUBLE:
return String.format("Argument -%c expects a double but was '%s'.",
errorArgumentId, errorParameter);
case MISSING_DOUBLE:
return String.format("Could not find double parameter for -%c.",
errorArgumentId);
}
return "";
}
public enum ErrorCode {
OK, INVALID_FORMAT, UNEXPECTED_ARGUMENT, INVALID_ARGUMENT_NAME,
MISSING_STRING,
MISSING_INTEGER, INVALID_INTEGER,
MISSING_DOUBLE, MISSING_BOOLEAN, INVALID_BOOLEAN, INVALID_DOUBLE}
}
}Args(본문)
public class Args {
private String schema;
private Map<Character, ArgumentMarshaler> marshalers = new HashMap<Character, ArgumentMarshaler>();
private Set<Character> argsFound = new HashSet<Character>();
private Iterator<String> currentArgument;
private List<String> argsList;
public Args(String schema, String[] args) throws ArgsException {
this.schema = schema;
argsList = Arrays.asList(args);
parse();
}
private void parse() throws ArgsException {
parseSchema();
parseArguments();
}
private boolean parseSchema() throws ArgsException {
for (String element : schema.split(",")) {
if (element.length() > 0) {
parseSchemaElement(element.trim());
}
}
return true;
}
private void parseSchemaElement(String element) throws ArgsException {
char elementId = element.charAt(0);
String elementTail = element.substring(1);
validateSchemaElementId(elementId);
if (elementTail.length() == 0)
marshalers.put(elementId, new BooleanArgumentMarshaler());
else if (elementTail.equals("*"))
marshalers.put(elementId, new StringArgumentMarshaler());
else if (elementTail.equals("#"))
marshalers.put(elementId, new IntegerArgumentMarshaler());
else if (elementTail.equals("##"))
marshalers.put(elementId, new DoubleArgumentMarshaler());
else
throw new ArgsException(ArgsException.ErrorCode.INVALID_FORMAT,
elementId, elementTail);
}
private void validateSchemaElementId(char elementId) throws ArgsException {
if (!Character.isLetter(elementId)) {
throw new ArgsException(ArgsException.ErrorCode.INVALID_ARGUMENT_NAME,
elementId, null);
}
}
private void parseArguments() throws ArgsException {
for (currentArgument = argsList.iterator(); currentArgument.hasNext();) {
String arg = currentArgument.next();
parseArgument(arg);
}
}
private void parseArgument(String arg) throws ArgsException {
if (arg.startsWith("-"))
parseElements(arg);
}
private void parseElements(String arg) throws ArgsException {
for (int i = 1; i < arg.length(); i++)
parseElement(arg.charAt(i));
}
private void parseElement(char argChar) throws ArgsException {
if (setArgument(argChar))
argsFound.add(argChar);
else {
throw new ArgsException(ArgsException.ErrorCode.UNEXPECTED_ARGUMENT,
argChar, null);
}
}
private boolean setArgument(char argChar) throws ArgsException {
ArgumentMarshaler m = marshalers.get(argChar);
if (m == null)
return false;
try {
m.set(currentArgument);
return true;
} catch (ArgsException e) {
e.setErrorArgumentId(argChar);
throw e;
}
}
public int cardinality() {
return argsFound.size();
}
public String usage() {
if (schema.length() > 0)
return "-[" + schema + "]";
else
return "";
}
public boolean getBoolean(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
boolean b = false;
try {
b = am != null && (Boolean) am.get();
} catch (ClassCastException e) {
b = false;
}
return b;
}
public String getString(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? "" : (String) am.get();
} catch (ClassCastException e) {
return "";
}
}
public int getInt(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Integer) am.get();
} catch (Exception e) {
return 0;
}
}
public double getDouble(char arg) {
ArgumentMarshaler am = marshalers.get(arg);
try {
return am == null ? 0 : (Double) am.get();
} catch (Exception e) {
return 0.0;
}
}
public boolean has(char arg) {
return argsFound.contains(arg);
}
}
