점진적인 개선

Hant·2022년 3월 9일
0

Clean Code

목록 보기
12/13
post-thumbnail

프로그램을 짜다 보면 종종 명령행 인수의 구문을 분석할 필요가 생깁니다. 편리한 유틸리티가 없다면 함수로 넘어오는 문자열 배열을 직접 분석하게 됩니다. 여러 가지 휼륭한 유틸리티가 있지만 내 사정에 딱 맞는 유틸리티가 없다면 직접 짜야합니다. 새로 짤 유틸리티를 Args라 부르겠습니다.

Args 사용법이 간단합니다. Args 생성자에 (입력으로 들어온) 인수 문자열과 형식 문자열을 넘겨 Args 인스턴스를 생성한 후 Args 인스턴스에다 인수 값을 질의합니다.

try {
  const arg = new Args("l,p#,d*", args);
  const logging = arg.getBoolean("l");
  const port = arg.getInt("p");
  const directory = arg.getStrging("d");
  executeApplication(logging, port, directory);
} catch (e) {
  console.error(`Argument error: ${e}`);
}

1. Args 구현

Typescript의 Iterator는 원문의 ListIterator에서 제공하는 previous, hasNext 등의 메서드를 제공하지 않습니다. 하여 커스텀 Iterator Class을 만들어서 사용했습니다.

class Args {
  private marshalers: Map<string, ArgumentMarshaler>;
  private argsFound: Set<string>;
  private currentArgument: ListIterator<string>;

  constructor(schema: string, args: string[]) {
    this.marshalers = new Map();
    this.argsFound = new Set();

    this.parseSchema(schema);
    this.parseArgumentStrings(args);
  }

  private parseSchema(schema: string) {
    for (const element of schema.split(",")) {
      if (element.length > 0) {
        this.parseSchemaElement(element);
      }
    }
  }

  private parseSchemaElement(element: string) {
    const elementId = element.charAt(0);
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (elementTail.length === 0) {
      this.marshalers.set(elementId, new BooleanArgumentMarshaler());
    } else if (elementTail === "*") {
      this.marshalers.set(elementId, new StringArgumentMarshaler());
    } else if (elementTail === "#") {
      this.marshalers.set(elementId, new NumberArgumentMarshaler());
    } else if (elementTail === "##") {
      this.marshalers.set(elementId, new BigIntArgumentMarshaler());
    } else if (elementTail === "[*]") {
      this.marshalers.set(elementId, new StringArrayArguemntMarshaler());
    } else {
      throw new ArgsException(ErrorCode.INVALID_ARGUMENT_FORMAT, elementTail, elementId);
    }
  }

  private validateSchemaElementId(elementId: string) {
    if (!/^\w$/i.test(elementId)) {
      throw new ArgsException(ErrorCode.INVALID_ARGUMENT_NAME, null, elementId);
    }
  }

  private parseArgumentStrings(argsList: string[]) {
    this.currentArgument = new ListIterator<string>(argsList);
    while (this.currentArgument.hasNext()) {
      const { value: argString } = this.currentArgument.next();
      if (argString.startsWith("-")) {
        this.parseArgumentCharacters(argString.slice(1));
      } else {
        this.currentArgument.previous();
        break;
      }
    }
  }

  private parseArgumentCharacters(argChars: string) {
    for (let i = 0; i < argChars.length; i += 1) {
      this.parseArgumentCharacter(argChars[i]);
    }
  }

  private parseArgumentCharacter(argChar: string) {
    const m = this.marshalers.get(argChar);
    if (m === undefined) {
      throw new ArgsException(ErrorCode.UNEXPECTED_ARGUMENT, null, argChar);
    }
    this.argsFound.add(argChar);
    try {
      m.set(this.currentArgument);
    } catch (e) {
      if (e instanceof ArgsException) {
        e.setErrorArgumentId(argChar);
      }
      throw e;
    }
  }

  has(arg: string): boolean {
    return this.argsFound.has(arg);
  }

  nextArgument(): number {
    return this.currentArgument.nextIndex();
  }

  getBoolean(arg: string) {
    return BooleanArgumentMarshaler.getValue(this.marshalers.get(arg));
  }

  getString(arg: string) {
    return StringArgumentMarshaler.getValue(this.marshalers.get(arg));
  }

  getNumber(arg: string) {
    return StringArgumentMarshaler.getValue(this.marshalers.get(arg));
  }

  getBigInt(arg: string) {
    return BigIntArgumentMarshaler.getValue(this.marshalers.get(arg));
  }

  getStringArray(arg: string) {
    return StringArrayArguemntMarshaler.getValue(this.marshalers.get(arg));
  }
}

class ListIterator<T> implements Iterator<T> {
  private readonly list: T[];
  private index: number;

  constructor(list: T[]) {
    this.list = [...list];
    this.index = 0;
  }

  next() {
    if (this.hasNext()) {
      const value = this.list[this.index];
      this.index += 1;
      return { value, done: false };
    }
    return { value: undefined, done: true };
  }

  hasNext() {
    return this.index < this.list.length;
  }

  previous() {
    if (this.hasPrevious()) {
      this.index -= 1;
      const value = this.list[this.index];
      return { value, done: false };
    }
    return { value: undefined, done: true };
  }

  hasPrevious() {
    return this.index > 0;
  }

  nextIndex() {
    return this.index;
  }
}

여기저기 뒤적일 필요 없이 위에서 아래로 코드가 읽힌다는 사실에 주목합니다. 아래는 ArgumentMarshaler 인터페이스와 파생 클래스입니다.

interface ArgumentMarshaler {
  set(currentArgument: Iterator<string>): void;
}

class BooleanArgumentMarshaler implements ArgumentMarshaler {
  private booeanVaule: boolean = false;

  set(currentArgument: Iterator<string>) {
    this.booeanVaule = true;
  }

  static getValue(am?: ArgumentMarshaler) {
    if (am !== undefined && am instanceof BooleanArgumentMarshaler) {
      return am.booeanVaule;
    }
    return false;
  }
}

class StringArgumentMarshaler implements ArgumentMarshaler {
  private stringValue: string = "";

  set(currentArgument: Iterator<string>) {
    this.stringValue = currentArgument.next().value;
    if (this.stringValue === undefined) {
      throw new ArgsException(ErrorCode.MISSING_STRING);
    }
  }

  static getValue(am?: ArgumentMarshaler) {
    if (am !== undefined && am instanceof StringArgumentMarshaler) {
      return am.stringValue;
    }
    return "";
  }
}

class NumberArgumentMarshaler implements ArgumentMarshaler {
  private numberValue: number;

  set(currentArgument: Iterator<string>) {
    const { value } = currentArgument.next();
    if (value === undefined) {
      throw new ArgsException(ErrorCode.MISSING_NUMBER);
    }

    this.numberValue = Number(value);
    if (Number.isNaN(this.numberValue)) {
      throw new ArgsException(ErrorCode.INVALID_NUMBER);
    }
  }

  static getValue(am?: ArgumentMarshaler) {
    if (am !== undefined && am instanceof NumberArgumentMarshaler) {
      return am.numberValue;
    }
    return 0;
  }
}

class BigIntArgumentMarshaler implements ArgumentMarshaler {
  private bigIntValue: BigInt;

  set(currentArgument: Iterator<string>) {
    const { value } = currentArgument.next();
    if (value === undefined) {
      throw new ArgsException(ErrorCode.MISSING_BIGINT);
    }

    try {
      this.bigIntValue = BigInt(value);
    } catch {
      throw new ArgsException(ErrorCode.INVALID_BIGINT);
    }
  }

  static getValue(am?: ArgumentMarshaler) {
    if (am !== undefined && am instanceof BigIntArgumentMarshaler) {
      return am.bigIntValue;
    }
    return 0n;
  }
}

class StringArrayArguemntMarshaler implements ArgumentMarshaler {
  private stringArrayValue: string[];

  set(currentArgument: Iterator<string>) {}

  static getValue(am?: ArgumentMarshaler) {
    if (am !== undefined && am instanceof StringArrayArguemntMarshaler) {
      return am.stringArrayValue;
    }
    return [];
  }
}

한 가지가 눈에 거슬릴지 모르겠습니다. 바로 오류 코드 상수를 정의하는 부분입니다.

class ArgsException extends Error {
  private errorArgumentId: string;
  private errorParameter: string;
  private errorCode: ErrorCode;

  static fromMessage(message: string) {
    const argsException = new ArgsException();
    argsException.message = message;
    return argsException;
  }

  constructor(errorCode?: ErrorCode, errorParameter?: string, errorArgumentId?: string) {
    super();
    this.errorArgumentId = errorArgumentId ?? "\0";
    this.errorParameter = errorParameter ?? null;
    this.errorCode = errorCode ?? ErrorCode.OK;
    this.message = this.makeErrorMessage();
  }

  getErrorArgumentId() {
    return this.errorArgumentId;
  }

  setErrorArgumentId(errorArgumentId: string) {
    this.errorArgumentId = errorArgumentId;
  }

  getErrorParameter() {
    return this.errorParameter;
  }

  setErrorParameter(errorParameter: string) {
    this.errorParameter = errorParameter;
  }

  getErrorCode() {
    return this.errorCode;
  }

  setErrorCode(errorCode: ErrorCode) {
    this.errorCode = errorCode;
  }

  private makeErrorMessage() {
    switch (this.errorCode) {
      case ErrorCode.OK:
        return "TILT: Should not get here.";
      case ErrorCode.UNEXPECTED_ARGUMENT:
        return `Argument -${this.errorArgumentId} unExpected.`;
      case ErrorCode.MISSING_STRING:
        return `Could not find string parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_NUMBER:
        return `Argument -${this.errorArgumentId} expects an number but was '${this.errorParameter}'.`;
      case ErrorCode.MISSING_NUMBER:
        return `Could not find number parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_BIGINT:
        return `Argument -${this.errorArgumentId} expects a BigInt but was '${this.errorParameter}'.`;
      case ErrorCode.MISSING_BIGINT:
        return `Could not find BigInt parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_ARGUMENT_NAME:
        return `'${this.errorArgumentId}' is not a valid argument name.`;
      case ErrorCode.INVALID_ARGUMENT_FORMAT:
        return `'${this.errorParameter}' is not a valid argument format.`;
      default:
        return this.message;
    }
  }
}

enum ErrorCode {
  OK,
  INVALID_ARGUMENT_FORMAT,
  UNEXPECTED_ARGUMENT,
  INVALID_ARGUMENT_NAME,
  MISSING_STRING,
  MISSING_NUMBER,
  INVALID_NUMBER,
  MISSING_BIGINT,
  INVALID_BIGINT,
}

이처럼 단순한 개념을 구현하는데 코드가 너무 많이 필요해 놀랄지도 모르겠습니다. 한 가지 이유는 타입스크립트를 사용해 정적인 타입 시스템을 만족시키려 했기 때문입니다. 자바스크립트로 동적 타입 시스템을 사용했다면 프로그램이 훨씬 작아졌을 것입니다.

2. Args: 1차 초안

class Args {
  private schema: string;
  private args: string[];
  private valid: boolean = true;
  private unExpectedArguments: Set<string> = new Set();
  private booleanArgs: Map<string, boolean> = new Map();
  private stringArgs: Map<string, string> = new Map();
  private numberArgs: Map<string, number> = new Map();
  private argsFound: Set<string> = new Set();
  private currentArgument: number;
  private errorArgumentId: string = "\0";
  private errorParameter: string = "TILT";
  private errorCode: ErrorCode = ErrorCode.OK;

  constructor(schema: string, args: string[]) {
    this.schema = schema;
    this.args = args;
    this.valid = this.parse();
  }

  private parse(): boolean {
    if (this.schema.length === 0 && this.args.length === 0) {
      return true;
    }
    this.parseSchema();
    try {
      this.parseArguments();
    } catch (e) {}
    return this.valid;
  }

  private parseSchema(): boolean {
    for (const element of this.schema.split(",")) {
      if (element.length > 0) {
        const trimmedElement = element.trim();
        this.parseSchemaElement(trimmedElement);
      }
    }
    return true;
  }

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (this.isBooleanSchemaElement(elementTail)) {
      this.parseBooleanSchemaElement(elementId);
    } else if (this.isStringSchemaElement(elementTail)) {
      this.parseStringSchemaElement(elementId);
    } else if (this.isNumberSchemaElement(elementTail)) {
      this.parseNumberSchemaElement(elementId);
    } else {
      throw new Error(`Argument: ${elementId} has invalid format: ${elementTail}.`);
    }
  }

  private validateSchemaElementId(elementId: string) {
    if (/^\w$/i.test(elementId)) {
      throw new Error(`Bad character: ${elementId} in Args format: ${this.schema}`);
    }
  }

  private parseBooleanSchemaElement(elementId: string) {
    this.booleanArgs.set(elementId, false);
  }

  private parseNumberSchemaElement(elementId: string) {
    this.numberArgs.set(elementId, 0);
  }

  private parseStringSchemaElement(elementId: string) {
    this.stringArgs.set(elementId, "");
  }

  private isStringSchemaElement(elementTail: string): boolean {
    return elementTail === "*";
  }

  private isBooleanSchemaElement(elementTail: string): boolean {
    return elementTail.length === 0;
  }

  private isNumberSchemaElement(elementTail: string): boolean {
    return elementTail === "#";
  }

  private parseArguments(): boolean {
    for (this.currentArgument = 0; this.currentArgument < this.args.length; this.currentArgument += 1) {
      const arg = this.args[this.currentArgument];
      this.parseArgument(arg);
    }
    return true;
  }

  private parseArgument(arg: string) {
    if (arg.startsWith("-")) {
      this.parseElements(arg);
    }
  }

  private parseElements(arg: string) {
    for (let i = 1; i < arg.length; i += 1) {
      this.parseElement(arg[i]);
    }
  }

  private parseElement(argChar: string) {
    if (this.setArgument(argChar)) {
      this.argsFound.add(argChar);
    } else {
      this.unExpectedArguments.add(argChar);
      this.errorCode = ErrorCode.UNEXPECTED_ARGUMENT;
      this.valid = false;
    }
  }

  private setArgument(argChar: string): boolean {
    if (this.isBooleanArg(argChar)) {
      this.setBooleanArg(argChar, true);
    } else if (this.isStringArg(argChar)) {
      this.setStringArg(argChar);
    } else if (this.isNumberArg(argChar)) {
      this.setNumberArg(argChar);
    } else {
      return false;
    }

    return true;
  }

  private isNumberArg(argChar: string): boolean {
    return this.numberArgs.has(argChar);
  }

  private setNumberArg(argChar: string) {
    this.currentArgument += 1;
    let parameter: number = null;
    if (this.currentArgument >= this.args.length) {
      this.valid = false;
      this.errorArgumentId = argChar;
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new this.ArgsException();
    }
    parameter = Number(this.args[this.currentArgument]);
    if (Number.isNaN(parameter)) {
      this.valid = false;
      this.errorArgumentId = argChar;
      this.errorParameter = this.args[this.currentArgument];
      this.errorCode = ErrorCode.INVALID_NUMBER;
      throw new this.ArgsException();
    }
    this.numberArgs.set(argChar, parameter);
  }

  private setStringArg(argChar: string) {
    this.currentArgument += 1;
    if (this.currentArgument >= this.args.length) {
      this.valid = false;
      this.errorArgumentId = argChar;
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new this.ArgsException();
    }
    this.stringArgs.set(argChar, this.args[this.currentArgument]);
  }

  private isStringArg(argChar: string): boolean {
    return this.stringArgs.has(argChar);
  }

  private setBooleanArg(argChar: string, value: boolean) {
    this.booleanArgs.set(argChar, value);
  }

  private isBooleanArg(argChar: string): boolean {
    return this.booleanArgs.has(argChar);
  }

  cardinality(): number {
    return this.argsFound.size;
  }

  usage(): string {
    if (this.schema.length > 0) {
      return `-[${this.schema}]`;
    } else {
      return "";
    }
  }

  errorMessage(): string {
    switch (this.errorCode) {
      case ErrorCode.OK:
        throw new Error("TILT: Should not get here.");
      case ErrorCode.UNEXPECTED_ARGUMENT:
        return this.unexpectedArgumentMessage();
      case ErrorCode.MISSING_STRING:
        return `Could not find string parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_NUMBER:
        return `Argument -${this.errorArgumentId} expects an number but was '${this.errorParameter}'.`;
      case ErrorCode.MISSING_NUMBER:
        return `Could not find number parameter for -${this.errorArgumentId}.`;
      default:
        return "";
    }
  }

  private unexpectedArgumentMessage(): string {
    let message = "Argument(s) -";
    for (const c of this.unExpectedArguments) {
      message += c;
    }
    message += " unexpected.";

    return message;
  }

  private falseIfNull(b?: boolean): boolean {
    return Boolean(b);
  }

  private zeroIfNull(i?: number): number {
    return i ?? 0;
  }

  private blankIfNull(s?: string): string {
    return s ?? "";
  }

  getString(arg: string): string {
    return this.blankIfNull(this.stringArgs.get(arg));
  }

  getNumber(arg: string): number {
    return this.zeroIfNull(this.numberArgs.get(arg));
  }

  getBoolean(arg: string): boolean {
    return this.falseIfNull(this.booleanArgs.get(arg));
  }

  has(arg: string): boolean {
    return this.argsFound.has(arg);
  }

  isValid(): boolean {
    return this.valid;
  }

  private ArgsException = class extends Error {};
}

enum ErrorCode {
  OK,
  UNEXPECTED_ARGUMENT,
  MISSING_STRING,
  MISSING_NUMBER,
  INVALID_NUMBER,
}

위 코드는 명백히 미완성입니다. 인스턴스 변수 개수만도 압도적입니다. 처음부터 지저분한 코드를 짜려는 생각은 없었습니다. 실제로도 코드를 어느 정도 손보려고 애썼습니다. 함수 이름이나 변수 이름을 선택한 방식, 어설프지만 나름대로 구조가 있다는 사실 등이 노력의 증거입니다. 하지만 어느 순간 프로그램은 손을 벗어났습니다.

코드는 조금씩 엉망이 되어갔습니다. 첫 버전은 이만큼 엉망이지는 않았습니다.

class Args {
  private schema: string;
  private args: string[];
  private valid: boolean = true;
  private unExpectedArguments: Set<string> = new Set();
  private booleanArgs: Map<string, boolean> = new Map();
  private numberOfArguments: number = 0;

  constructor(schema: string, args: string[]) {
    this.schema = schema;
    this.args = args;
    this.valid = this.parse();
  }

  isValid(): boolean {
    return this.valid;
  }

  private parse(): boolean {
    if (this.schema.length === 0 && this.args.length === 0) {
      return true;
    }
    this.parseSchema();
    this.parseArguments();
    return this.unExpectedArguments.size === 0;
  }

  private parseSchema(): boolean {
    for (const element of this.schema.split(".")) {
      this.parseSchemaElement(element);
    }
    return true;
  }

  private parseSchemaElement(element: string) {
    if (element.length === 1) {
      this.parseBooleanSchemaElement(element);
    }
  }

  private parseBooleanSchemaElement(element: string) {
    const c = element[0];
    if (/^\w$/i.test(element)) {
      this.booleanArgs.set(c, false);
    }
  }

  private parseArguments(): boolean {
    for (const arg of this.args) {
      this.parseArgument(arg);
    }
    return true;
  }

  private parseArgument(arg: string) {
    if (arg.startsWith("-")) {
      this.parseElements(arg);
    }
  }

  private parseElements(arg: string) {
    for (let i = 1; i < arg.length; i += 1) {
      this.parseElement(arg[i]);
    }
  }

  private parseElement(argChar: string) {
    if (this.isBoolean(argChar)) {
      this.numberOfArguments += 1;
      this.setBooleanArg(argChar, true);
    } else {
      this.unExpectedArguments.add(argChar);
    }
  }

  private setBooleanArg(argChar: string, value: boolean) {
    this.booleanArgs.set(argChar, value);
  }

  private isBoolean(argChar: string) {
    return this.booleanArgs.has(argChar);
  }

  cardinality(): number {
    return this.numberOfArguments;
  }

  usage(): string {
    if (this.schema.length > 0) {
      return `-[${this.schema}]`;
    } else {
      return "";
    }
  }

  errorMessage(): string {
    if (this.unExpectedArguments.size > 0) {
      return this.unexpectedArgumentMessage();
    }
    return "";
  }

  private unexpectedArgumentMessage(): string {
    let message = "Argument(s) -";
    for (const c of this.unExpectedArguments) {
      message += c;
    }
    message += " unexpected.";

    return message;
  }

  getBoolean(arg: string): boolean {
    return this.booleanArgs.get(arg);
  }
}

위 코드도 불평거리가 많겠지만 나름대로 괜찮은 코드입니다. 간결하고 단순하며 이해하기도 쉽습니다. 하지만 코드를 잘 살펴보면 나중에 엉망으로 변해갈 씨앗이 보입니다. 코드가 점차 지저분해진 이유가 분명히 드러납니다.

뒤에 나올 코드는 위 코드에 stringnumber라는 인수 유형 두 개만 추가햇을 뿐이라는 사실에 주목합니다. 인수 유형 두 개만 더했을 뿐인데 코드가 엄청나게 지저분해졌습니다. 유지와 보수가 수월했던 코드가 버그와 결함이 숨어있을지도 모른다는 상당히 의심스러운 코드로 뒤바뀌었습니다.

먼저 string 인수를 추가해 다음 코드를 얻었습니다.

class Args {
  private schema: string;
  private args: string[];
  private valid: boolean = true;
  private unExpectedArguments: Set<string> = new Set();
  private booleanArgs: Map<string, boolean> = new Map();
  private stringArgs: Map<string, string> = new Map();
  private argsFound: Set<string> = new Set();
  private currentArgument: number;
  private errorArgument: string = "\0";
  private errorCode = ErrorCode.OK;

  constructor(schema: string, args: string[]) {
    this.schema = schema;
    this.args = args;
    this.valid = this.parse();
  }

  private parse(): boolean {
    if (this.schema.length === 0 && this.args.length === 0) {
      return true;
    }
    this.parseSchema();
    this.parseArguments();
    return this.valid;
  }

  private parseSchema(): boolean {
    for (const element of this.schema.split(".")) {
      if (element.length > 0) {
        const trimmedElement = element.trim();
        this.parseSchemaElement(trimmedElement);
      }
    }
    return true;
  }

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (this.isBooleanSchemaElement(elementTail)) {
      this.parseBooleanSchemaElement(elementId);
    } else if (this.isStringSchemaElement(elementTail)) {
      this.parseStringSchemaElement(elementId);
    }
  }

  private validateSchemaElementId(elementId: string) {
    if (/^\w$/i.test(elementId)) {
      throw new Error(`Bad character: ${elementId} in Args format: ${this.schema}`);
    }
  }

  private parseStringSchemaElement(elementId: string) {
    this.stringArgs.set(elementId, "");
  }

  private isStringSchemaElement(elementTail: string): boolean {
    return elementTail === "*";
  }

  private isBooleanSchemaElement(elementTail: string): boolean {
    return elementTail.length === 0;
  }

  private parseBooleanSchemaElement(elementId: string) {
    this.booleanArgs.set(elementId, false);
  }

  private parseArguments(): boolean {
    for (this.currentArgument = 0; this.currentArgument < this.args.length; this.currentArgument += 1) {
      const arg = this.args[this.currentArgument];
      this.parseArgument(arg);
    }
    return true;
  }
  private parseArgument(arg: string) {
    if (arg.startsWith("-")) {
      this.parseElements(arg);
    }
  }

  private parseElements(arg: string) {
    for (let i = 1; i < arg.length; i += 1) {
      this.parseElement(arg[i]);
    }
  }

  private parseElement(argChar: string) {
    if (this.setArgument(argChar)) {
      this.argsFound.add(argChar);
    } else {
      this.unExpectedArguments.add(argChar);
      this.valid = false;
    }
  }

  private setArgument(argChar: string): boolean {
    let set: boolean = true;
    if (this.isBooleanArg(argChar)) {
      this.setBooleanArg(argChar, true);
    } else if (this.isStringArg(argChar)) {
      this.setStringArg(argChar);
    } else {
      set = false;
    }

    return set;
  }

  private setStringArg(argChar: string) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      this.stringArgs.set(argChar, this.args[this.currentArgument]);
    } else {
      this.valid = false;
      this.errorArgument = argChar;
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }

  private isStringArg(argChar: string): boolean {
    return this.stringArgs.has(argChar);
  }

  private setBooleanArg(argChar: string, value: boolean) {
    this.booleanArgs.set(argChar, value);
  }

  private isBooleanArg(argChar: string): boolean {
    return this.booleanArgs.has(argChar);
  }

  cardinality(): number {
    return this.argsFound.size;
  }

  usage(): string {
    if (this.schema.length > 0) {
      return `-[${this.schema}]`;
    } else {
      return "";
    }
  }

  errorMessage(): string {
    if (this.unExpectedArguments.size > 0) {
      return this.unexpectedArgumentMessage();
    }
    switch (this.errorCode) {
      case ErrorCode.OK:
        throw new Error("TILT: Should not get here.");
      case ErrorCode.MISSING_STRING:
        return `Could not find string parameter for -${this.errorArgument}.`;
      default:
        return "";
    }
  }

  private unexpectedArgumentMessage(): string {
    let message = "Argument(s) -";
    for (const c of this.unExpectedArguments) {
      message += c;
    }
    message += " unexpected.";

    return message;
  }

  getBoolean(arg: string): boolean {
    return this.falseIfNull(this.booleanArgs.get(arg));
  }

  private falseIfNull(b?: boolean): boolean {
    return Boolean(b);
  }

  getString(arg: string): string {
    return this.blankIfNull(this.stringArgs.get(arg));
  }

  private blankIfNull(s?: string): string {
    return s ?? "";
  }

  has(arg: string): boolean {
    return this.argsFound.has(arg);
  }

  isValid(): boolean {
    return this.valid;
  }
}

enum ErrorCode {
  OK,
  MISSING_STRING,
}

보다시피 코드는 통제를 벗어나기 시작했습니다. 여기저기 눈에 거슬리지만 아직은 엉망이라 부르기 어렵습니다. 여기다 number 인수 유형을 추가하니 코드는 완전히 엉망이 되어버렸습니다.

그래서 멈췄다

추가할 인수 유형이 적어도 두 개는 더 있었는데 그러면 코드가 훨씬 더 나빠지리라는 사실이 저명했습니다. 그래서 기능을 더 이상 추가하지 않기로 결정하고 리펙터링을 시작했습니다. string 인수와 number 인수 유형을 추가한 경혐에서 새 인수 유형을 추가하려면 주요 지점 세 곳에다 코드를 추가해야 한다는 사실을 깨달았습니다.

  1. 인수 유형에 해당하는 Map을 선택하기 위해 스키마 요소의 구문을 분석했습니다.
  2. 명령행 인수에서 인수 유형을 분석해 진짜 유형으로 변환합니다.
  3. getXXX 메서를 구현해 호출자에게 진짜 유형을 반환합니다.

인수 유형은 다양하지만 모두가 유사한 메서드를 제공하므로 클래스 하나가 적합하다고 판단했습니다. 그래서 ArgumentMarshaler라는 개념이 탄생했습니다.

점진적으로 개선하다

프로그램을 망치는 가장 좋은 방법 중 하나는 개선이라는 이름 아래 구조를 크게 뒤집는 행위입니다. 어떤 프로그램 그저 그런 개선에서 결코 회복하지 못합니다. 개선 전과 똑같은 프로그램을 돌리기가 아주 어렵기 떄문입니다. 그래서 테스트 주도 개발(Test-Driven Development, TDD)이라는 기법을 사용했습니다. TDD는 언제 어느 때라도 시스템이 돌아가야 한다는 원칙을 따릅니다.

시스템의 자잘한 변경을 가하기 시작했습니다. 코드를 변경할 떄마다 시스템 구조는 조금씩 ArgumentMarsharler 개념에 가까워 졌습니다. 또한 변경 후에도 시스템은 변경 전과 다름없이 돌아갔습니다. 가장 먼저 코드 ArgumentMarshaler 클래스의 골격을 추가했습니다.

class ArgumentMarshaler {
  private booleanValue: boolean = false;

  setBoolean(value: boolean) {
    this.booleanValue = value;
  }

  getBoolean() {
    return this.booleanValue;
  }
}

class BooleanArgumentMarshaler extends ArgumentMarshaler {}

class StringArgumentMarshaler extends ArgumentMarshaler {}

class NumberArgumentMarshaler extends ArgumentMarshaler {}

당연히 위와 같은 변경은 아무 문제도 일으키지 않습니다. 다음으로 코드를 최소로 건드리는, 가장 단순한 변경을 가했습니다. 구체적으로 boolean 인수를 저장하는 Map에서 boolean 인수 유형을 ArgumentMarshaler 유형으로 바꿨습니다.

private booleanArgs: Map<string, ArgumentMarshaler> = new Map();

// ...

privaet parseBooleanSchemaElement(elementId: string) {
  this.booleanArgs.set(elementId, new BooleanArgumentMarshaler());
}

// ...

private setBooleanArg(argChar: string, value: boolean) {
  this.booleanArgs.get(argChar).setBoolean(value);
}

// ...

getBoolean(arg: string): boolean {
  return this.falseIfNull(this.booleanArgs.get(arg).getBoolean());
}

앞서 세 인수 유형을 추가하려면 세 곳(parse, get, set)을 변경해야 한다고 말했는데, 위에서 수정한 부분과 정확히 일치한다는 사실에 주목합니다. 불행히도 사소한 변경이었으나 몇몇 테스트 케이스가 실패하기 시작했습니다. getBoolean 함수를 자세히 살펴봅니다. y라는 인수가 없는데 argsy를 넘긴다면 booleanArgs.get('y')undefined를 반환하고 예기치 못한 상황이 발생할 수 있습니다.

이제 undefined인지 확인할 객체는 boolean이 아니라 ArgumentMarshaler입니다. 가장 먼저 getBoolean 함수에서 falseNull을 제거했습니다.

getBoolean(arg: string): boolean {
  return this.booleanArgs.get(arg).getBoolean();
}

다음으로는 함수를 두 행으로 쪼갠 후 ArgumentMarshalerargumentMarshaler라는 독자적인 변수에 저장했습니다. 그렇지만 긴 변수 이름이 싫었습니다. 유형 이름과 중복이 심했고 함수가 길어졌습니다. 그래서 argumentMarshaleram으로 줄였습니다. 그런다음 undefined를 점검했습니다.

getBoolean(arg: string): boolean {
  const am = this.booleanArgs.get(arg);
  return am !== undefined && am.getBoolean();
}

3. String 인수

string 인수를 추가하는 과정은 boolean 인수와 매우 유사합니다. Map을 변경한 후 parse, set, get 함수를 고쳤습니다.

class Args {
  // ...

  private stringArgs: Map<string, ArgumentMarshaler> = new Map();

  // ...

  private parseStringSchemaElement(elementId: string) {
    this.stringArgs.set(elementId, new StringArgumentMarshaler());
  }

  private setStringArg(argChar: string) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      this.stringArgs.get(argChar).setString(this.args[this.currentArgument]);
    } else {
      this.valid = false;
      this.errorArgument = argChar;
      this.errorCode = ErrorCode.MISSING_STRING;
      throw new Error();
    }
  }

  getString(arg: string): string {
    const am = this.stringArgs.get(arg);
    return am === undefined ? "" : am.getString();
  }
}

class ArgumentMarshaler {
  private booleanValue: boolean = false;
  private stringValue: string;

  setBoolean(value: boolean) {
    this.booleanValue = value;
  }

  getBoolean() {
    return this.booleanValue;
  }

  setString(s: string) {
    this.stringValue = s;
  }

  getString() {
    return this.stringValue ?? "";
  }
}

일단 각 인수 유형을 처리하는 코드를 모두 ArgumentMarshaler 클래스에 넣고 나서 ArgumentMarshlaer 파생 클래스를 만들어 코드를 분리할 생각입니다. 그러면 프로그램 구조를 조금씩 변경하는 동안에도 시스템의 정상 동작을 유지하기 쉬워집니다. 다음으로 number 인수 기능을 ArgumentMarshaler로 옯겼습니다.

class Args {
  // ...

  private numberArgs: Map<string, ArgumentMarshaler> = new Map();

  // ...

  private parseNumberSchemaElement(elementId: string) {
    this.numberArgs.set(elementId, new NumberArgumentMarshaler());
  }

  // ...

  private setNumberArg(argChar: string) {
    this.currentArgument += 1;
    let parameter = null;
    try {
      parameter = this.args[this.currentArgument];
      this.numberArgs.get(argChar).setNumber(Number(parameter));
    } catch {
      this.valid = false;
      this.errorArgument = argChar;
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }

  // ...

  getNumber(arg: string): number {
    const am = this.numberArgs.get(arg);
    return am === undefined ? 0 : am.getNumber();
  }
}

class ArgumentMarshaler {
  // ...

  private numberValue: number;

  // ...

  setNumber(i: number) {
    this.numberValue = i;
  }

  getNumber() {
    return this.numberValue;
  }
}

이제 모든 논리를 ArgumentMarshaler로 옮겼으니 파생 클래스를 만들어 기능을 분산할 차례입니다.

class Args {
  // ...

  private setBooleanArg(argChar: string, value: boolean) {
    this.booleanArgs.get(argChar).set("true");
  }

  private setStringArg(argChar: string) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      this.stringArgs.get(argChar).set(this.args[this.currentArgument]);
    } else {
      this.valid = false;
      this.errorArgument = argChar;
      this.errorCode = ErrorCode.MISSING_STRING;
      throw new Error();
    }
  }

  private setNumberArg(argChar: string) {
    this.currentArgument += 1;
    let parameter = null;
    if (this.currentArgument < this.args.length) {
      try {
        parameter = this.args[this.currentArgument];
        this.numberArgs.get(argChar).set(parameter);
      } catch (e) {
        this.valid = false;
        this.errorArgument = argChar;
        this.errorCode = ErrorCode.INVALID_NUMBER;
        throw new Error();
      }
    } else {
      this.valid = false;
      this.errorArgument = argChar;
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }

  // ...

  getBoolean(arg: string): boolean {
    const am = this.booleanArgs.get(arg);
    return am !== undefined && (am.get() as boolean);
  }

  getString(arg: string): string {
    const am = this.stringArgs.get(arg);
    return am === undefined ? "" : (am.get() as string);
  }

  getNumber(arg: string): number {
    const am = this.numberArgs.get(arg);
    return am === undefined ? 0 : (am.get() as number);
  }
}

abstract class ArgumentMarshaler {
  abstract set(s: string): void;
  abstract get(): any;
}

class BooleanArgumentMarshaler extends ArgumentMarshaler {
  private booleanValue: boolean = false;

  set(s: string) {
    this.booleanValue = true;
  }

  get() {
    return this.booleanValue;
  }
}

class StringArgumentMarshaler extends ArgumentMarshaler {
  private stringValue: string = "";

  set(s: string) {
    this.stringValue = s;
  }

  get() {
    return this.stringValue;
  }
}

class NumberArgumentMarshaler extends ArgumentMarshaler {
  private numberValue: number = 0;

  set(s: string) {
    this.numberValue = Number(s);
    if (Number.isNaN(this.numberValue)) {
      throw new Error();
    }
  }

  get() {
    return this.numberValue;
  }
}

다음응로 알고리즘 처음에 나오는 맵 세 개를 없앴습니다. 그러면 전체 시스템이 훨씬 더 일반적으로 변합니다. ArgumentMarshaler로 맵을 만들어 원래 맵을 교체하고 관련 메서드를 변경합니다.

class Args {
  // ...

  private marshalers: Map<string, ArgumentMarshaler> = new Map();

  // ...

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (this.isBooleanSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new BooleanArgumentMarshaler());
    } else if (this.isStringSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new StringArgumentMarshaler());
    } else if (this.isNumberSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new NumberArgumentMarshaler());
    } else {
      throw new Error(`Argument: ${elementId} ahs invalid format: ${elementTail}.`);
    }
  }

  private parseBooleanSchemaElement(elementId: string) {
    this.marshalers.set(elementId, new BooleanArgumentMarshaler());
  }

  private parseStringSchemaElement(elementId: string) {
    this.marshalers.set(elementId, new StringArgumentMarshaler());
  }

  private parseNumberSchemaElement(elementId: string) {
    this.marshalers.set(elementId, new NumberArgumentMarshaler());
  }

  // ...

  private setArgument(argChar: string): boolean {
    const m = this.marshalers.get(argChar);
    try {
      if (m instanceof BooleanArgumentMarshaler) {
        this.setBooleanArg(m);
      } else if (m instanceof StringArgumentMarshaler) {
        this.setStringArg(m);
      } else if (m instanceof NumberArgumentMarshaler) {
        this.setNumberArg(m);
      } else {
        return false;
      }
    } catch (e) {
      this.valid = false;
      this.errorArgument = argChar;
      throw e;
    }

    return true;
  }

  private setBooleanArg(m: BooleanArgumentMarshaler) {
    m.set("true");
  }

  private setStringArg(m: StringArgumentMarshaler) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      m.set(this.args[this.currentArgument]);
    } else {
      this.errorCode = ErrorCode.MISSING_STRING;
      throw new Error();
    }
  }

  private setNumberArg(m: NumberArgumentMarshaler) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      try {
        const parameter = this.args[this.currentArgument];
        m.set(parameter);
      } catch (e) {
        this.errorCode = ErrorCode.INVALID_NUMBER;
        throw e;
      }
    } else {
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }

  // ...

  getBoolean(arg: string): boolean {
    const am = this.marshalers.get(arg);
    return am !== undefined && typeof am.get() === "boolean" && am.get();
  }

  getString(arg: string): string {
    const am = this.marshalers.get(arg);
    return am === undefined || typeof am.get() !== "string" ? "" : am.get();
  }

  getNumber(arg: string): number {
    const am = this.marshalers.get(arg);
    return am === undefined || typeof am.get() !== "number" ? 0 : am.get();
  }
}

이제 전체 그림을 한 번 더 돌아볼 차례입니다.

enum ErrorCode {
  OK,
  MISSING_STRING,
  MISSING_NUMBER,
  INVALID_NUMBER,
}

class Args {
  private schema: string;
  private args: string[];
  private valid: boolean = true;
  private unExpectedArguments: Set<string> = new Set();
  private marshalers: Map<string, ArgumentMarshaler> = new Map();
  private argsFound: Set<string> = new Set();
  private currentArgument: number;
  private errorArgumentId: string = "\0";
  private errorParameter: string = "TILT";
  private errorCode = ErrorCode.OK;

  constructor(schema: string, args: string[]) {
    this.schema = schema;
    this.args = args;
    this.valid = this.parse();
  }

  private parse(): boolean {
    if (this.schema.length === 0 && this.args.length === 0) {
      return true;
    }
    this.parseSchema();
    this.parseArguments();
    return this.valid;
  }

  private parseSchema(): boolean {
    for (const element of this.schema.split(",")) {
      if (element.length > 0) {
        const trimmedElement = element.trim();
        this.parseSchemaElement(trimmedElement);
      }
    }
    return true;
  }

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (this.isBooleanSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new BooleanArgumentMarshaler());
    } else if (this.isStringSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new StringArgumentMarshaler());
    } else if (this.isNumberSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new NumberArgumentMarshaler());
    } else {
      throw new Error(`Argument: ${elementId} ahs invalid format: ${elementTail}.`);
    }
  }

  private validateSchemaElementId(elementId: string) {
    if (!/^\w$/i.test(elementId)) {
      throw new Error(`Bad character: ${elementId} in Args format: ${this.schema}`);
    }
  }

  private isBooleanSchemaElement(elementTail: string): boolean {
    return elementTail.length === 0;
  }

  private isStringSchemaElement(elementTail: string): boolean {
    return elementTail === "*";
  }

  private isNumberSchemaElement(elementTail: string) {
    return elementTail === "#";
  }

  private parseArguments(): boolean {
    for (this.currentArgument = 0; this.currentArgument < this.args.length; this.currentArgument += 1) {
      const arg = this.args[this.currentArgument];
      this.parseArgument(arg);
    }
    return true;
  }

  private parseArgument(arg: string) {
    if (arg.startsWith("-")) {
      this.parseElements(arg);
    }
  }

  private parseElements(arg: string) {
    for (let i = 1; i < arg.length; i += 1) {
      this.parseElement(arg[i]);
    }
  }

  private parseElement(argChar: string) {
    if (this.setArgument(argChar)) {
      this.argsFound.add(argChar);
    } else {
      this.unExpectedArguments.add(argChar);
      this.valid = false;
    }
  }

  private setArgument(argChar: string): boolean {
    const m = this.marshalers.get(argChar);
    try {
      if (m instanceof BooleanArgumentMarshaler) {
        this.setBooleanArg(m);
      } else if (m instanceof StringArgumentMarshaler) {
        this.setStringArg(m);
      } else if (m instanceof NumberArgumentMarshaler) {
        this.setNumberArg(m);
      } else {
        return false;
      }
    } catch (e) {
      this.valid = false;
      this.errorArgumentId = argChar;
      throw e;
    }

    return true;
  }

  private setBooleanArg(m: BooleanArgumentMarshaler) {
    m.set("true");
  }

  private setStringArg(m: StringArgumentMarshaler) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      m.set(this.args[this.currentArgument]);
    } else {
      this.errorCode = ErrorCode.MISSING_STRING;
      throw new Error();
    }
  }

  private setNumberArg(m: NumberArgumentMarshaler) {
    this.currentArgument += 1;
    if (this.currentArgument < this.args.length) {
      try {
        const parameter = this.args[this.currentArgument];
        m.set(parameter);
      } catch (e) {
        this.errorCode = ErrorCode.INVALID_NUMBER;
        throw e;
      }
    } else {
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }

  cardinality(): number {
    return this.argsFound.size;
  }

  usage(): string {
    if (this.schema.length > 0) {
      return `-[${this.schema}]`;
    } else {
      return "";
    }
  }

  errorMessage(): string {
    if (this.unExpectedArguments.size > 0) {
      return this.unexpectedArgumentMessage();
    }
    switch (this.errorCode) {
      case ErrorCode.OK:
        throw new Error("TILT: Should not get here.");
      case ErrorCode.UNEXPECTED_ARGUMENT:
        return this.unexpectedArgumentMessage();
      case ErrorCode.MISSING_STRING:
        return `Could not find string parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_NUMBER:
        return `Argument ${this.errorArgumentId} expects an number but was ${this.errorParameter}`;
      case ErrorCode.MISSING_NUMBER:
        return `Could not number parameter for ${this.errorArgumentId}`;
      default:
        return "";
    }
  }

  private unexpectedArgumentMessage(): string {
    let message = "Argument(s) -";
    for (const c of this.unExpectedArguments) {
      message += c;
    }
    message += " unexpected.";

    return message;
  }

  getBoolean(arg: string): boolean {
    const am = this.marshalers.get(arg);
    return am !== undefined && typeof am.get() === "boolean" && am.get();
  }

  getString(arg: string): string {
    const am = this.marshalers.get(arg);
    return am === undefined || typeof am.get() !== "string" ? "" : am.get();
  }

  getNumber(arg: string): number {
    const am = this.marshalers.get(arg);
    return am === undefined || typeof am.get() !== "number" ? 0 : am.get();
  }

  has(arg: string): boolean {
    return this.argsFound.has(arg);
  }

  isValid(): boolean {
    return this.valid;
  }
}

abstract class ArgumentMarshaler {
  abstract set(s: string): void;
  abstract get(): any;
}

class BooleanArgumentMarshaler extends ArgumentMarshaler {
  private booleanValue: boolean = false;

  set(s: string) {
    this.booleanValue = true;
  }

  get() {
    return this.booleanValue;
  }
}

class StringArgumentMarshaler extends ArgumentMarshaler {
  private stringValue: string = "";

  set(s: string) {
    this.stringValue = s;
  }

  get() {
    return this.stringValue;
  }
}

class NumberArgumentMarshaler extends ArgumentMarshaler {
  private numberValue: number = 0;

  set(s: string) {
    this.numberValue = Number(s);
    if (Number.isNaN(this.numberValue)) {
      throw new Error();
    }
  }

  get() {
    return this.numberValue;
  }
}

열심히 고쳤지만 결과는 다소 실망스럽습니다. 구조만 조금 나아졌을 뿐입니다. 첫머리에 나오는 변수는 그대로 남아있으며, setArgument에는 유형을 일일이 확인하는 보기 싫은 코드도 그대로 남아있습니다. 게다가 모든 set 함수는 정말로 흉합니다. 오류 처리 코드도 마찬가지입니다.

setArgument 함수에서 유형을 일일이 확인하는 코드를 없애고, ArgumentMarshaler.set만 호출하면 충분하게 만들고 싶습니다. 즉, setNumberArg, setStringArg, setBooleanArg을 해당 ArgumentMarshaler 파생 클래스로 내려야 한다는 뜻입니다. 하지만 문제가 있습니다.

setNumberArg를 자세히 살펴보면 argscountArgument라는 인스턴스 변수 두 개가 쓰입니다. setNumberArgNumberArgumentMarshaler로 내리려면 argscurrnetArgument를 인수로 넘겨야 한다는 말입니다. 그러면 코드가 지저분해집니다. 인수를 하나만 남기는 편이 낫습니다. 다행스럽게도 해결책은 간단합니다. args 배열을 list로 변환한 후 iteratorset 함수로 전달하면 됩니다.

class Args {
  // ...

  private currentArgument: ListIterator<string>;

  // ...

  private parseArguments(): boolean {
    for (this.currentArgument = new ListIterator<string>(this.args); this.currentArgument.hasNext(); ) {
      const { value: arg } = this.currentArgument.next();
      this.parseArgument(arg);
    }

    return true;
  }

  // ...

  private setStringArg(m: StringArgumentMarshaler) {
    if (this.currentArgument.hasNext()) {
      const { value: parameter } = this.currentArgument.next();
      m.set(parameter);
    } else {
      this.errorCode = ErrorCode.MISSING_STRING;
      throw new Error();
    }
  }

  private setNumberArg(m: NumberArgumentMarshaler) {
    let parameter = null;
    if (this.currentArgument.hasNext()) {
      try {
        const { value } = this.currentArgument.next();
        parameter = value;
        m.set(parameter);
      } catch (e) {
        this.errorParameter = parameter;
        this.errorCode = ErrorCode.INVALID_NUMBER;
        throw e;
      }
    } else {
      this.errorCode = ErrorCode.MISSING_NUMBER;
      throw new Error();
    }
  }
}

class ListIterator<T> implements Iterator<T> {
  private readonly list: T[];
  private index: number;

  constructor(list: T[]) {
    this.list = [...list];
    this.index = 0;
  }

  next() {
    if (this.hasNext()) {
      const value = this.list[this.index];
      this.index += 1;
      return { value, done: false };
    }
    return { value: undefined, done: true };
  }

  hasNext() {
    return this.index < this.list.length;
  }

  previous() {
    if (this.hasPrevious()) {
      this.index -= 1;
      const value = this.list[this.index];
      return { value, done: false };
    }
    return { value: undefined, done: true };
  }

  hasPrevious() {
    return this.index > 0;
  }

  nextIndex() {
    return this.index;
  }
}

이제는 set 함수를 적절한 파생 클래스로 내려도 괜찮아졌습니다. 리펙터링을 하다 보면 코드를 넣었다 뺏다 하는 사례가 아주 흔합니다. 단계적으로 조금씩 변경하며 매번 테스트를 돌려야 하므로 코드를 여기저기 옮길 일이 많아집니다. 리펙터링은 루빅 큐브 맞추기와 비슷합니다. 큰 목표를 하나 이루기 위해 자잘한 단계를 수없이 거칩니다. 각 단계를 거쳐야 다음 단계가 가능합니다.

class Args {
  // ...

  private setArgument(argChar: string): boolean {
    const m = this.marshalers.get(argChar);
    if (m === null) {
      return false;
    }

    try {
      m.set(this.currentArgument);
      return true;
    } catch (e) {
      this.valid = false;
      this.errorArgumentId = argChar;
      throw e;
    }

    return true;
  }

  // ...
}

interface ArgumentMarshaler {
  set(currentArgument: ListIterator<string>): void;
  get(): any;
}

class BooleanArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    this.booleanValue = true;
  }

  //...
}

class StringArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      // ErrorCode.MISSING_STRING
      throw new Error();
    }

    this.stringValue = currentArgument.next().value;
  }

  // ...
}

class NumberArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      // ErrorCode.MISSING_NUMBER;
      throw new Error();
    }

    const { value } = currentArgument.next();
    this.numberValue = Number(value);

    if (Number.isNaN(this.numberValue)) {
      // ErrorCode.INVALID_NUMBER
      throw new Error();
    }
  }

  // ...
}

이제부터 새로운 인수 유형을 추가하는 일은 매우 쉽습니다. 변경할 코드는 아주 적으며 나머지 시스켐에 영향을 미치지 않습니다. BigInt 유형을 제대로 받아들이는지 확인할 테스트 케이스부터 추가합니다.

function testSimpleDoublePresent() {
  const args = new Args("x##", ["-x", "9007199254740991"]);
  expect(args.isValid()).toBeTrue();
  expect(args.cardinality()).toBe(1);
  expect(args.has("x")).toBeTrue();
  expect(args.getBigInt("x")).toBe(9007199254740991n);
}

이제 스키마 구문분석 코드를 정리하고 ## 감지 코드를 추가합니다.

enum ErrorCode {
  // ...
  MISSING_BIGINT,
  INVALID_BIGINT,
}

class Args {
  // ...

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (this.isBooleanSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new BooleanArgumentMarshaler());
    } else if (this.isStringSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new StringArgumentMarshaler());
    } else if (this.isNumberSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new NumberArgumentMarshaler());
    } else if (this.isBigIntSchemaElement(elementTail)) {
      this.marshalers.set(elementId, new BigIntArgumentMarshaler());
    } else {
      throw new Error(`Argument: ${elementId} ahs invalid format: ${elementTail}.`);
    }
  }

  // ...

  private isBigIntSchemaElement(elementTail: string) {
    return elementTail === "##";
  }

  // ...

  getBigInt(arg: string): BigInt {
    const am = this.marshalers.get(arg);
    return am === undefined || typeof am.get() !== "bigint" ? 0n : am.get();
  }

  // ...
}

class BigIntArgumentMarshaler implements ArgumentMarshaler {
  private bigIntValue: BigInt = 0n;

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      // ErrorCode.MISSING_BIGINT;
      throw new Error();
    }

    try {
      const { value } = currentArgument.next();
      this.bigIntValue = BigInt(value);
    } catch {
      // ErrorCode.INVALID_BIGINT
      throw new Error();
    }
  }

  get() {
    return this.bigIntValue;
  }
}

혅재까지 예외 코드는 아주 흉할뿐더러 사실상 Args 클래스에 속하지도 않습니다. 게다가 ParseException은 Args 클래스에 속하지 않습니다. 그러므로 모든 예외를 하나로 모아 ArgsException 클래스를 만든 후 독자 모듈로 옮깁니다.

export default class ArgsException extends Error {
  private errorArgumentId: string;
  private errorParameter: string;
  private errorCode: ErrorCode;

  static fromMessage(message: string) {
    const argsException = new ArgsException();
    argsException.message = message;
    return argsException;
  }

  constructor(errorCode?: ErrorCode, errorParameter?: string, errorArgumentId?: string) {
    super();
    this.errorArgumentId = errorArgumentId ?? "\0";
    this.errorParameter = errorParameter ?? null;
    this.errorCode = errorCode ?? ErrorCode.OK;
    this.message = this.makeErrorMessage();
  }

  getErrorArgumentId() {
    return this.errorArgumentId;
  }

  setErrorArgumentId(errorArgumentId: string) {
    this.errorArgumentId = errorArgumentId;
  }

  getErrorParameter() {
    return this.errorParameter;
  }

  setErrorParameter(errorParameter: string) {
    this.errorParameter = errorParameter;
  }

  getErrorCode() {
    return this.errorCode;
  }

  setErrorCode(errorCode: ErrorCode) {
    this.errorCode = errorCode;
  }

  private makeErrorMessage() {
    switch (this.errorCode) {
      case ErrorCode.OK:
        return "TILT: Should not get here.";
      case ErrorCode.UNEXPECTED_ARGUMENT:
        return `Argument -${this.errorArgumentId} unExpected.`;
      case ErrorCode.MISSING_STRING:
        return `Could not find string parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_NUMBER:
        return `Argument -${this.errorArgumentId} expects an number but was '${this.errorParameter}'.`;
      case ErrorCode.MISSING_NUMBER:
        return `Could not find number parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_BIGINT:
        return `Argument -${this.errorArgumentId} expects a BigInt but was '${this.errorParameter}'.`;
      case ErrorCode.MISSING_BIGINT:
        return `Could not find BigInt parameter for -${this.errorArgumentId}.`;
      case ErrorCode.INVALID_ARGUMENT_NAME:
        return `'${this.errorArgumentId}' is not a valid argument name.`;
      case ErrorCode.INVALID_FORMAT:
        return `'${this.errorParameter}' is not a valid argument format.`;
      default:
        return this.message;
    }
  }
}

export enum ErrorCode {
  OK,
  INVALID_FORMAT,
  INVALID_ARGUMENT_NAME,
  UNEXPECTED_ARGUMENT,
  MISSING_STRING,
  MISSING_NUMBER,
  MISSING_BIGINT,
  INVALID_NUMBER,
  INVALID_BIGINT,
}
class Args {
  private schema: string;
  private argsList: string[];
  private marshalers: Map<string, ArgumentMarshaler> = new Map();
  private argsFound: Set<string> = new Set();
  private currentArgument: ListIterator<string>;

  constructor(schema: string, args: string[]) {
    this.schema = schema;
    this.argsList = [...args];
    this.parse();
  }

  private parse() {
    this.parseSchema();
    this.parseArguments();
  }

  private parseSchema() {
    for (const element of this.schema.split(",")) {
      if (element.length > 0) {
        this.parseSchemaElement(element.trim());
      }
    }
  }

  private parseSchemaElement(element: string) {
    const elementId = element[0];
    const elementTail = element.slice(1);
    this.validateSchemaElementId(elementId);
    if (elementTail.length === 0) {
      this.marshalers.set(elementId, new BooleanArgumentMarshaler());
    } else if (elementTail === "*") {
      this.marshalers.set(elementId, new StringArgumentMarshaler());
    } else if (elementTail === "#") {
      this.marshalers.set(elementId, new NumberArgumentMarshaler());
    } else if (elementTail === "##") {
      this.marshalers.set(elementId, new BigIntArgumentMarshaler());
    } else {
      throw new ArgsException(ErrorCode.INVALID_FORMAT, elementTail, elementId);
    }
  }

  private validateSchemaElementId(elementId: string) {
    if (!/^\w$/i.test(elementId)) {
      throw new ArgsException(ErrorCode.INVALID_ARGUMENT_NAME, null, elementId);
    }
  }

  private parseArguments() {
    this.currentArgument = new ListIterator<string>(this.argsList);
    while (this.currentArgument.hasNext()) {
      const { value: arg } = this.currentArgument.next();
      this.parseArgument(arg);
    }
  }

  private parseArgument(arg: string) {
    if (arg.startsWith("-")) {
      this.parseElements(arg);
    }
  }

  private parseElements(arg: string) {
    for (let i = 1; i < arg.length; i += 1) {
      this.parseElement(arg[i]);
    }
  }

  private parseElement(argChar: string) {
    if (this.setArgument(argChar)) {
      this.argsFound.add(argChar);
    } else {
      throw new ArgsException(ErrorCode.UNEXPECTED_ARGUMENT, null, argChar);
    }
  }

  private setArgument(argChar: string): boolean {
    const m = this.marshalers.get(argChar);
    if (m === null) {
      return false;
    }

    try {
      m.set(this.currentArgument);
      return true;
    } catch (e) {
      if (e instanceof ArgsException) {
        e.setErrorArgumentId(argChar);
      }
      throw e;
    }
  }

  // ...
}

class StringArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      throw new ArgsException(ErrorCode.MISSING_STRING);
    }

    this.stringValue = currentArgument.next().value;
  }

  // ...
}

class NumberArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      throw new ArgsException(ErrorCode.MISSING_NUMBER);
    }

    const { value } = currentArgument.next();
    this.numberValue = Number(value);

    if (Number.isNaN(this.numberValue)) {
      throw new ArgsException(ErrorCode.INVALID_NUMBER, value);
    }
  }

  // ...
}

class BigIntArgumentMarshaler implements ArgumentMarshaler {
  // ...

  set(currentArgument: ListIterator<string>) {
    if (!currentArgument.hasNext()) {
      throw new ArgsException(ErrorCode.MISSING_BIGINT);
    }

    let parameter: string | null = null;
    try {
      parameter = currentArgument.next().value;
      this.bigIntValue = BigInt(parameter);
    } catch {
      throw new ArgsException(ErrorCode.MISSING_BIGINT, parameter);
    }
  }

  // ...
}

눈여겨볼 코드는 ArgsExceptionerrorMessage 메서드입니다. 기존 Args에 속했던 이 메서드는 명백히 SRP(Single Responsibility Principle) 위반이었습니다. Args 클래스가 오류 메시지 형식까지 처리하는 클래스가 아니기 때문입니다. 하지만 그렇다고 ArgsException 클래스가 오류 메시지 형식을 처리해야 옳은걸까요?

솔직히 말해, 이것은 절충안입니다. ArgsException에게 맡겨서는 안 된다고 생각하는 독자라면 새로운 클래스가 필요합니다. 하지만 미리 깔끔하게 만들어진 오류 메시지로 얻는 장점은 무시하기 어렵습니다.

4. 결론

그저 돌아가는 코드만으로는 부족합니다. 나쁜 코드보다 더 오랫동안 더 심각하게 개발 프로젝트에 악영향을 미치는 요인도 없습니다. 물론 나쁜 코드도 깨끗한 코드로 개선할 수 있습니다. 하지만 비용이 엄청나게 많이 듭니다. 반면 처음부터 코드를 깨끗하게 유지하기란 상대적으로 쉽습니다. 그러므로 코드는 언제나 최대한 깔끔하고 단순하게 정리합니다.

5. 출처

  • 제목: 클린 코드 - 애자일 소프트웨어 장인 정신
  • 저자: 로버트 C.마틴
  • 옮긴이: 박재호, 이해영
  • 출판사: 인사이트
profile
끊임없이 도전하는 프론트 개발자가 되고자 노력합니다.

0개의 댓글