-
클린 코드 - 점진적 개선프로그래밍/방법론 2021. 10. 21. 18:23반응형
args 파싱 프로그램
Java시절 사용하던 main 함수의 args 배열 파라미터를 Args라는 인스턴스에 값을 저장하고 쿼리하는 내용의 프로그램을 작성해보자.
class MainActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_main) try { val schema = "l,p#,d*" val args = arrayOf("-p", "10", "-d", "Hi", "-l", "true") val arg = Args(schema, args) val logging = arg.getBoolean('l') val port = arg.getInt('p') val directory = arg.getString('d') Log.d("TAG", "onCreate: $logging $port $directory") // onCreate: true 10 Hi // executeApplication(logging, port, directory); } catch (e: ArgsException) { try { System.out.printf("Argument error: %s\n", e.errorMessage()) } catch (exception: Exception) { exception.printStackTrace() } } } }
- 위와 같이 "l, p#, d*" 이라는 스키마가 있고 "," 구분자로 구분된다.
- Schema(형식 문자열) - args의 키 값이 될 예정이다.
- "l" - 문자 끝(Tail)에 아무것도 없으면 Boolean을 의미
- "p#" - 문자 끝(Tail)에 #이 있으면 Integer을 의미
- "d*" - 문자 끝(Tail)에 *이 있으면 String을 의미
- 파라미터 - arrayOf("-p", "10", "-d", "Hi", "-l", "true") 라는 파라미터는
- 10,
- Hi,
- true
- 라는 값이 될 것이다.
- Map이 있다면,
- <"I", Boolean(true)>
- <"p", Integer(10)>
- <"d", String("Hi")>`
- 와 같이 저장될 파싱 프로그램을 만들 것이다.
- Map이 있다면,
형식 문자열이나 파라미터에 문제가 있다면 ArgsException이 발생한다.
Args 구현
처음에는 초안을 작성해야한다. 깨끗하고 우아한 프로그램은 한방에 뚝딱 나오지 않는다.
학창시절에 작문을 할 때도, 1차 초안 -> 2차 초안 -> 최종안을 만들어왔다.
Args 1차 초안
처음에 짰던 코드이다. 이 코드는 돌아가지만 엉망이다. 이 코드는 HashSets와 TreeSets, try-catch- catch 블록 등 모두가 지저분한 코드에 기여하는 요인이다.
코드는 점점 엉망이 되어간다.
다음은 Boolean만 지원하는 첫 버전 코드이다.
- scheme = "a,b,c"
- args = "-a", "-c""와 같은 식으로 아래 코드에 입력되면, 아래와 같이 매핑된다.
- booleanArgs : Map<Charactor, Boolean>
- <"a", Boolean(true)>
- <"b", Boolean(false)>
- <"c", Boolean(true)>`
package com.linecorp.cleancodes14.onlyBoolean; import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.TreeSet; public class Args { private final String schema; private final String[] args; private final boolean valid; private final Set<Character> unexpectedArguments = new TreeSet<Character>(); private final Map<Character, Boolean> booleanArgs = new HashMap<Character, Boolean>(); private int numberOfArguments = 0; public Args(String schema, String[] args) { this.schema = schema; this.args = args; valid = parse(); } public boolean isValid() { return valid; } private boolean parse() { if (schema.length() == 0 && args.length == 0) return true; parseSchema(); parseArguments(); return unexpectedArguments.size() == 0; } private boolean parseSchema() { for (String element : schema.split(",")) { parseSchemaElement(element); } return true; } private void parseSchemaElement(String element) { if (element.length() == 1) { parseBooleanSchemaElement(element); } } private void parseBooleanSchemaElement(String element) { char c = element.charAt(0); if (Character.isLetter(c)) { booleanArgs.put(c, false); } } private boolean parseArguments() { for (String arg : args) parseArgument(arg); return true; } private void parseArgument(String arg) { if (arg.startsWith("-")) parseElements(arg); } private void parseElements(String arg) { for (int i = 1; i < arg.length(); i++) parseElement(arg.charAt(i)); // -"a" } private void parseElement(char argChar) { if (isBoolean(argChar)) { numberOfArguments++; setBooleanArg(argChar, true); } else unexpectedArguments.add(argChar); } private void setBooleanArg(char argChar, boolean value) { booleanArgs.put(argChar, value); } private boolean isBoolean(char argChar) { return booleanArgs.containsKey(argChar); } public int cardinality() { return numberOfArguments; } public String usage() { if (schema.length() > 0) return "-[" + schema + "]"; else return ""; } public String errorMessage() { if (unexpectedArguments.size() > 0) { return unexpectedArgumentMessage(); } else return ""; } private String unexpectedArgumentMessage() { StringBuffer message = new StringBuffer("Argument(s) -"); for (char c : unexpectedArguments) { message.append(c); } message.append(" unexpected."); return message.toString(); } public boolean getBoolean(char arg) { return booleanArgs.get(arg); } }
나름 Boolean 쪽 코드만 있어서 볼만하다. 하지만 나중에 이 코드는 String과 Integer라는 유형 두 개만 추가되는데도 지저분해진다.
다음은 String 유형이 추가된 코드이다.
booleanArgs만 있을 때는 보기가 좋았는데 stringArgs도 추가된다.
- scheme = "a,b*,c*"
- args = "-a", "true", "-b", "Hello", "-c", "World"와 같은 식으로 아래 코드에 입력되면, 아래와 같이 매핑된다.
- booleanArgs : Map<Charactor, Boolean>
- <"a", Boolean(true)>
- stringArgs: Map<Charactor, String>
- <"b", String("Hello")>
- <"c", String("World")>`
package com.linecorp.cleancodes14.onlyBoolean.plusStr; import java.text.ParseException; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.TreeSet; public class Args { private String schema; private String[] args; private boolean valid = true; private Set<Character> unexpectedArguments = new TreeSet<Character>(); private Map<Character, Boolean> booleanArgs = new HashMap<Character, Boolean>(); private Map<Character, String> stringArgs = new HashMap<Character, String>(); // 추가됨 private Set<Character> argsFound = new HashSet<Character>(); private int currentArgument; private char errorArgument = '\0'; enum ErrorCode { // 추가됨 OK, MISSING_STRING } private ErrorCode errorCode = ErrorCode.OK; public Args(String schema, String[] args) throws ParseException { this.schema = schema; this.args = args; valid = parse(); } private boolean parse() throws ParseException { if (schema.length() == 0 && args.length == 0) return true; parseSchema(); parseArguments(); return valid; } private boolean parseSchema() throws ParseException { for (String element : schema.split(",")) { if (element.length() > 0) { String trimmedElement = element.trim(); parseSchemaElement(trimmedElement); } } return true; } 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); // 추가됨 } private void validateSchemaElementId(char elementId) throws ParseException { if (!Character.isLetter(elementId)) { throw new ParseException( "Bad character:" + elementId + "in Args format: " + schema, 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 void parseBooleanSchemaElement(char elementId) { booleanArgs.put(elementId, false); } private boolean parseArguments() { // 추가됨 for (currentArgument = 0; currentArgument < args.length; currentArgument++) { String arg = args[currentArgument]; parseArgument(arg); } return true; } private void parseArgument(String arg) { if (arg.startsWith("-")) parseElements(arg); } private void parseElements(String arg) { for (int i = 1; i < arg.length(); i++) parseElement(arg.charAt(i)); } private void parseElement(char argChar) { if (setArgument(argChar)) argsFound.add(argChar); else { unexpectedArguments.add(argChar); valid = false; } } private boolean setArgument(char argChar) { boolean set = true; if (isBoolean(argChar)) setBooleanArg(argChar, true); else if (isString(argChar)) // 추가됨 setStringArg(argChar, ""); else set = false; return set; } private void setStringArg(char argChar, String s) { // 추가됨 currentArgument++; try { stringArgs.put(argChar, args[currentArgument]); } catch (ArrayIndexOutOfBoundsException e) { valid = false; errorArgument = argChar; errorCode = ErrorCode.MISSING_STRING; } } private boolean isString(char argChar) { return stringArgs.containsKey(argChar); } private void setBooleanArg(char argChar, boolean value) { booleanArgs.put(argChar, value); } private boolean isBoolean(char argChar) { return booleanArgs.containsKey(argChar); } public int cardinality() { return argsFound.size(); } public String usage() { if (schema.length() > 0) return "-[" + schema + "]"; else return ""; } public String errorMessage() throws Exception { if (unexpectedArguments.size() > 0) { return unexpectedArgumentMessage(); } else switch (errorCode) { case MISSING_STRING: return String.format("Could not find string parameter for -%c.", errorArgument); case OK: throw new Exception("TILT: Should not get here."); } return ""; } private String unexpectedArgumentMessage() { StringBuffer message = new StringBuffer("Argument(s) -"); for (char c : unexpectedArguments) { message.append(c); } message.append(" unexpected."); return message.toString(); } public boolean getBoolean(char arg) { return falseIfNull(booleanArgs.get(arg)); } private boolean falseIfNull(Boolean b) { return b == null ? false : b; } public String getString(char arg) { // 추가됨 return blankIfNull(stringArgs.get(arg)); } private String blankIfNull(String s) { return s == null ? "" : s; } public boolean has(char arg) { return argsFound.contains(arg); } public boolean isValid() { return valid; } }
그래서 멈췄다
추가할 유형이 두 가지 더 있었는데 그러면 코드가 훨씬 더 나빠질 것이기 때문에 구조를 변경하기로 판단했다.
그래서 기능을 더이상 추가하지 않고 리팩토링을 시작했다.
- 첫째, 스키마 요소와 파라미터에 어떤 타입들이 있는지 분석해 HashMap을 어떻게 구성할지 생각한다.
- booleanArgs, stringArgs 등 지저분한데 합칠 방법은 없을까?
- " " -> Boolean, "*" -> String, "#" -> Integer
- 둘째, 파라미터에서 타입을 분석해 진짜 타입으로 변환한다.
- 인터페이스로 통합하고 실제 타입으로 구현한다.
- 셋째, getXXX 메소드를 구현해 호출자에게 진짜 유형을 반환한다.
파라미터 타입은 여러 개지만 모두가 유사한 메소드를 제공하므로 하나의 클래스가 적합하다 판단했다. 그래서 `ArgumentMarshaler`라는 개념이 탄생했다.
점진적 개선
- ArgumentMarshaler 추상 클래스 추가
- booleanArgs: Map<Character, Boolean>, stringArgs: Map<Character, String>
- -> marshalers: Map<Character, ArgumentMarshaler> 로 통일
개선이라는 이름으로 구조를 크게 뒤집는 행동은 프로그램을 망치는 행동이다. 따라서 테스트를 먼저 작성하고 변경을 하더라도 언제든지 동일한 동작을 해야한다. Args 클래스를 구현하는 동안 테스트 케이스를 함께 작성해야한다. 언제든지 테스트가 자동으로 동작하도록 해야한다.
이제 ArgumentMarshaler 클래스의 골격을 추가해보자.
private abstract class ArgumentMarshaler { private boolean booleanValue = false; public void setBoolean(boolean value) { booleanValue = value; } public boolean getBoolean() { return booleanValue; } // Boolean, String, Integer private class BooleanArgumentMarshaler extends ArgumentMarshaler private class StringArgumentMarshaler extends ArgumentMarshaler { private String stringValue = ""; public void setString(String value) { stringValue = value; } public String getStringValue() { return stringValue; } } private class IntegerArgumentMarshaler extends ArgumentMarshaler { } }
우선 booleanArgs의 HashMap의 파라미터 타입을 ArgumentMarshaler로 바꿨다.
private Map<Character, ArgumentMarshaler> booleanArgs = new HashMap<Character, ArgumentMarshaler>(); ... private void parseBooleanSchemaElement(char elementld) { booleanArgs.put(elementld , new BooleanArgumentMarshaler()) }; private void setBooleanArg(char argChar, boolean value) { booleanArgs.get(argChar).setBoolean(value); } public boolean getBoolean(char arg){ return falselIfNull(booleanArgs.get(arg).getBoolean()); }
String, Integer도 동일하게 Marshaler 클래스로 변경했다. 파라미터 타입이 Marshaler 클래스로 통일되면서 하나의 Map만 갖도록 할 수 있다.
private Map<Character, ArgumentMarshaler> marshalers = new HashMap<Character, ArgumentMarshaler>();
parse코드를 한줄짜리 코드들로 만들 수 있다.
- 각 타입별 HashMap에서,
- booleanArgs = <"I", Boolean(true)>
- IntegerArgs = <"p", Integer(10)>
- StringArgs = <"d", String("Hi")> 라는 값으로 저장했을 때,
- 이제 Marshalers타입으로 통일할 수 있다.
- marshalers에서,
- <"I", BooleanArgumentMarshaler()>
- <"p", integerArgumentMarshaler()>
- <"d", StringArgumentMarshaler()>
- marshalers에서,
private boolean parseSchema() throws ParseException { for (String element : schema.split(",")) { if (element.length() > 0) { String trimmedElement = element.trim(); parseSchemaElement(trimmedElement); } } return true; } private void parseSchemaElement(String element) throws ParseException { char elementId = element.charAt(0); String elementTail = element.substring(1); validateSchemaElementId(elementId); if (isBooleanSchemaElement(elementTail)) marshalers.put(elementId, new BooleanArgumentMarshaler()); else if (isStringSchemaElement(elementTail)) marshalers.put(elementId, new StringArgumentMarshaler()); else if (isIntegerSchemaElement(elementTail)) { marshalers.put(elementId, new IntegerArgumentMarshaler()); } else { throw new ParseException(String.format( "Argument: %c has invalid format: %s.", elementId, elementTail), 0); } }
marshalers로부터 파라미터를 Get할 수 있도록 getString(), getInt()도 추가되었다.
- marshalers에서,
- <"I", Boolean(true)>
- <"p", Integer(10)>
- <"d", String("Hi")>`
- getString(d) -> "HI", getInt(p) -> 10 을 가져올 수 있다.
public String getString(char arg) { Args.ArgumentMarshaler am = marshalers.get(arg); try { return am == null ? "" : (String) am.get(); } catch (ClassCastException e) { return ""; } } public int getInt(char arg) { Args.ArgumentMarshaler am = marshalers.get(arg); try { return am == null ? 0 : (Integer) am.get(); } catch (Exception e) { return 0; } }
String, Integer의 Marshaler에 Arg를 추가할 수 있도록 setIntegerArg(), setStringArg()도 추가되었다.
- setIntegerArg(), setStringArg()를 이용하면,
- 타입별 인스턴스만 있는 marshalers에서,
- <"I", BooleanArgumentMarshaler()>
- <"p", integerArgumentMarshaler()>
- <"d", StringArgumentMarshaler()>
- 라고 스키마 먼저 저장되어있으면 키에 해당하는 값을 찾아와,
- <"I", BooleanArgumentMarshaler(true)>
- <"p", integerArgumentMarshaler(10)>
- <"d", StringArgumentMarshaler("Hi")>`
- 라고 저장된다.
private boolean setArgument(char argChar) throws ArgsException { ArgumentMarshaler m = marshalers.get(argChar); try { if (m instanceof BooleanArgumentMarshaler) setBooleanArg(m); else if (m instanceof StringArgumentMarshaler) setStringArg(m); else if (m instanceof IntegerArgumentMarshaler) setIntArg(m); else return false; } catch (ArgsException e) { valid = false; errorArgumentId = argChar; throw e; } return true; } private void setIntArg(ArgumentMarshaler m) throws ArgsException { currentArgument++; String parameter = null; try { parameter = args[currentArgument]; m.set(parameter); } catch (ArrayIndexOutOfBoundsException e) { errorCode = ErrorCode.MISSING_INTEGER; throw new ArgsException(); } catch (ArgsException e) { errorParameter = parameter; errorCode = ErrorCode.INVALID_INTEGER; throw e; } } private void setStringArg(ArgumentMarshaler m) throws ArgsException { currentArgument++; try { m.set(args[currentArgument]); } catch (ArrayIndexOutOfBoundsException e) { errorCode = ErrorCode.MISSING_STRING; throw new ArgsException(); } }
그에 따라 에러메시지도 더 추가되었다.
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 ""; }
마지막 리팩토링
- currentArgument 를 Iterator로 변경
지금으로서도 괜찮은 것 같지만, setArgument는 일일히 타입을 확인하는 정말 보기 싫은 코드가 됐다.
private boolean setArgument(char argChar) throws ArgsException { ArgumentMarshaler m = marshalers.get(argChar); try { if (m instanceof BooleanArgumentMarshaler) setBooleanArg(m); else if (m instanceof StringArgumentMarshaler) setStringArg(m); else if (m instanceof IntegerArgumentMarshaler) setIntArg(m); else return false; } catch (ArgsException e) { valid = false; errorArgumentId = argChar; throw e; } return true; }
이 코드를 없애고 ArgumentMarshaler. set만 호출하면 충분히 가능할 것 같다. 이렇게 변경하려면 setlntArg, setStringArg, setBooleanArg를 각 구현으로 내려야한다.
setIntArg를 IntArgumentMarshaler로 내리려면 args와 currentArgs를 인수로 넘겨야 한다는 소리다.
private void setIntArg(ArgumentMarshaler m) throws ArgsException { currentArgument++; String parameter = null; try { parameter = args[currentArgument]; // arg를 가져오는 부분을 구현별로 내려야한다. m.set(parameter); } catch (ArrayIndexOutOfBoundsException e) { errorCode = ErrorCode.MISSING_INTEGER; throw new ArgsException(); } catch (ArgsException e) { errorParameter = parameter; errorCode = ErrorCode.INVALID_INTEGER; throw e; } }
인수를 하나만 넘기는 편이 낫다. 다행스럽게도 해결책은 간단 하다. args 배열을 list로 변환한 후 Iterator를 set 힘수로 전달하면 된다.
private Iterator<String> currentArgument; // 이터레이터와 private List<String> argsList; // 리스트로 만든다. public Args(String schema, String[] args) throws ArgsException { this.schema = schema; argsList = Arrays.asList(args); parse(); } // Before //private boolean parseArguments() throws ArgsException { // for (currentArgument = 0; currentArgument < args.length; currentArgument++) { // String arg = args[currentArgument]; // ["-p", 10] -> "-p" // parseArgument(arg); // } // return true; //} // After private void parseArguments() throws ArgsException { for (currentArgument = argsList.iterator(); currentArgument.hasNext(); ) { String arg = currentArgument.next(); // ["-p", 10] -> "-p" parseArgument(arg); } } private void parseArgument(String arg) throws ArgsException { if (arg.startsWith("-")) parseElements(arg); } private void parseElement(char argChar) throws ArgsException { if (setArgument(argChar)) ... } // Before //private boolean setArgument(char argChar) throws ArgsException { // ArgumentMarshaler m = marshalers.get(argChar); // try { // if (m instanceof BooleanArgumentMarshaler) setBooleanArg(m); // else if (m instanceof StringArgumentMarshaler) setStringArg(m); // else if (m instanceof IntegerArgumentMarshaler) setIntArg(m); // else // return false; // } catch (ArgsException e) { // valid = false; // errorArgumentId = argChar; // throw e; // } // return true; //} //private void setIntArg(ArgumentMarshaler m) throws ArgsException { // currentArgument++; // ["-p", 10] -> 10. Iterator로 바꿔서 무슨 index인지 알필요 없어짐. // String parameter = null; // try { // parameter = args[currentArgument]; // m.set(parameter); // } catch (ArrayIndexOutOfBoundsException e) { // errorCode = ErrorCode.MISSING_INTEGER; // throw new ArgsException(); // } catch (ArgsException e) { // errorParameter = parameter; // errorCode = ErrorCode.INVALID_INTEGER; // throw e; // } //} // After private boolean setArgument(char argChar) throws ArgsException { ArgumentMarshaler m = marshalers.get(argChar); if (m == null) return false; try { m.set(currentArgument.next()); // ["-p", 10] -> 10 return true; } catch (ArgsException e) { e.setErrorArgumentId(argChar); throw e; } } private abstract class ArgumentMarshaler { public void set(String s) throws ArgsException; public Object get(); } private class IntegerArgumentMarshaler extends ArgumentMarshaler { private int intValue = 0; public void set(String s) throws ArgsException { try { intValue = Integer.parseInt(s); // 10 } catch (NumberFormatException e) { throw new ArgsException(); } } public Object get() { return intValue; } }
여기서 ArgumentMarshaler클래스는 메소드 파라미터와 리턴 타입이 Object로 바뀌고, 구현이 없어지면서 interface로 변경되어도 된다.
private interface ArgumentMarshaler { public abstract void set(String s) throws ArgsException; public abstract Object get(); }
결론
나쁜 코드보다 더 오랫동안 더 심각하게 개발 프로젝트에 악영향을 미치는 요인도 없다. 나쁜 일정은 다시 짜면 된다. 나쁜 요구사항은 다시 정의하면 된다. 나쁜 팀 역학은 복구하면 된다. 하지만 나쁜 코드는 썩어 문드러진다. 점점 무게가 늘어나 팀의 발목을 잡는다. 속력이 점점 느려지다 못해 기어가는 팀도 많이 보았다. 너무 서두르다가 이후로 영원히 자신들의 운명을 지배할 악성 코드라는 굴레를 젊어진다.
처음부터 코드를 깨끗하게 유지하기란 상대적으로 쉽다. 아침에 엉망으로 만든 코드를 오후에 정리하기는 어렵지 않다. 더욱이 5분 전에 엉망으로 만든 코드는 지금 당장 정리하기 아주 쉽다.
위의 예제로부터 생각해볼 수 있는 결론,
- 인풋 타입, 아웃풋 타입을 생각해 타입을 공통으로 합칠 수 있으면 합친다. (스키마, 파라미터 타입들..)
- 여러 타입의 클래스가 생길 것 같지만 유사한 메소드가 있다면 하나의 인터페이스로 통일한다.
- 유사한 클래스 필드 타입을 변경해 좋은 코드를 유지한다.
반응형'프로그래밍 > 방법론' 카테고리의 다른 글
도메인 주도 설계 - 도메인 주도 설계란? (0) 2023.01.24 클린 코드 - 단위 테스트 (0) 2021.08.27 클린 코드 - 주석 (0) 2021.07.26 네이버 면접시 듣게 되는 41가지 질문 (0) 2017.06.19 진정한 프로그래머는 알고리즘머이다 (0) 2014.11.16