study cleancode 책너두 점진적인 개선 리팩토링

점진적인 개선(2)

초안 살펴보기

책너두 5기 29일차

로버트 C. 마틴의 클린코드 p. 255~ p.272

내용정리

14. 점진적인 개선

초안 작성

Args.java (Boolean만 지원하는 버전)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113

package com.objectmentor.utilities.getopts;

import java.util.*;

public class Args {
  private String schema;
  private String[] args;
  private boolean valid;
  private Set<Character> unexpectedArguments = new TreeSet<Character>();
  private 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));
  }

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

가장 초기의 코드. 우선 String 인수 유형을 추가하여 아래 코드가 된다.

Args.java(Boolean과 String)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202

 package com.objectmentor.utilities.getopts;

   import java.text.ParseException;
   import java.util.*;

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

코드가 엉망이 되어 더이상 추가하는 것을 멈췄다.

그래서 멈췄다.

더 이상 인수 유형 추가를 멈추고 리팩터링을 한다. 새 인수 유형을 추가하려면 주요 지점 세 곳에다 코드를 추가해야 한다.(parse, get, set)

  1. 인수 유형에 해당하는 HashMap을 선택하기 위해 스키마 요소의 구문을 분석한다. (14번 행)
  2. 명령행 인수에서 인수 유형을 분석해 진짜 유형으로 변환한다.(24번 행 쯤)
  3. getXXX 메서드를 구현해 호출자에게 진짜 유형을 반환한다. (187번 행)

인수 유형은 다양하지만 모두가 유사한 메서드를 제공하므로 클래스 하나가 적합하다고 판단하여 ArgumentMarshaler를 만들었다.

점진적으로 개선하다

개선한다고 구조를 크게 뒤집는 행위는 프로그램을 망친다. 개선 전과 똑같이 프로그램을 돌리기 아주 어렵기 떄문이다.

TDD(Test-Driven Development) 기법은 언제 어느 때라도 시스템이 돌아가야 한다는 원칙을 따르기 때문에 이럴 때 유용하다. 처음 Args 클래스를 구현할 때 만들어 놓은 단위 테스트 슈트와 인수 테스트를 반복해소 실행하여 변경하는 동안 시스템이 테스트를 통과하는 것을 확인해야 한다.

이 과정에서 기존 코드 끝에 ArgumentMarshaler를 추가한다.

Args.java 끝에 추가한 ArgumentMarshaler

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

private class ArgumentMarshaler {
       private boolean booleanValue = false;


       public void setBoolean(boolean value) {
         booleanValue = value;
       }

       public boolean getBoolean() {return booleanValue;}
     }

     private class BooleanArgumentMarshaler extends ArgumentMarshaler {
     }

     private class StringArgumentMarshaler extends ArgumentMarshaler {
     }

     private class IntegerArgumentMarshaler extends ArgumentMarshaler {
     }
   }

아래와 같이 변경.

1
2

private Map<Character, ArgumentMarshaler> booleanArgs =
     new HashMap<Character, ArgumentMarshaler>();

그러나 코드 일부가 깨져서 빠르게 고침.

1
2
3
4
5
6
7
8
9
10
11
12

…
     private void parseBooleanSchemaElement(char elementId) {
       booleanArgs.put(elementId, new BooleanArgumentMarshaler());
     }
   ..
     private void setBooleanArg(char argChar, boolean value) {
       booleanArgs.get(argChar).setBoolean(value);
     }
   …
     public boolean getBoolean(char arg) {
       return falseIfNull(booleanArgs.get(arg).getBoolean());
   }

앞에서 말한 새 인수 유형을 추가하기 위해 변경해야할 3곳(parge, get, set)과 위 수정 부분이 정확히 일치함을 알 수 있다.

여기서 테스트가 또 실패하는 것을 확인하고 분석하였다. getBoolean함수는 y 인수가 없는데 args로 y를 넘긴다면 booleanArgs, get(y)는 null을 반환하고 함수는 NullPointerException을 던진다. 코드가 수정되면서 falseIfNull 함수가 제 기능을 못하여 생긴 문제였다.

따라서 null 점검 위치를 바꾸기 위해 null 확인 객체를 boolean이 아니라 ArgumentMarshaler로 바꾸었다. 우선 falseIfNull이 쓸모가 없으므로 지운다.

1
2
3

public boolean getBoolean(char arg) {
     return booleanArgs.get(arg).getBoolean();
   }

함수를 두행으로 쪼개 ArgumentMarshaler와 argumentMarshaler 라는 독자적인 변수에 저장했다. 이름이 너무 길어 am으로 줄인다.

1
2
3
4

public boolean getBoolean(char arg) {
     Args.ArgumentMarshaler am = booleanArgs.get(arg);
     return am.getBoolean();
   }

이제 null을 점검한다.

1
2
3
4

public boolean getBoolean(char arg) {
     Args.ArgumentMarshaler am = booleanArgs.get(arg);
     return am != null && am.getBoolean();
   }

읽고 나서

기존 코드를 리팩토링 할때 나의 문제점은 작동되는지를 뒷전에 둔다는 것이다. TDD 기법을 통해 항상 어느 순간에도 테스트가 실행되는 코드를 유지하면서도 충분히 리팩토링 할 수 있다. 이를 명심하자.

CATALOG
    cleancode