Чистый код. Создание, анализ и рефакторинг — страница 44 из 94

  private class IntegerArgumentMarshaler extends ArgumentMarshaler {
    private int intValue = 0;
    public void set(String s) throws ArgsException {
      try {
        intValue = Integer.parseInt(s);
      } catch (NumberFormatException e) {
        throw new ArgsException();
      }
    }
    public Object get() {
      return intValue;
    }
  }
}

Вроде бы проделана большая работа, а результат не впечатляет. Структура кода немного улучшилась, но в начале листинга по-прежнему объявляются многочисленные переменные; в setArgument осталась кошмарная конструкция проверки типа; функции set выглядят просто ужасно. Я уже не говорю об обработке ошибок… Нам еще предстоит большая работа.

Прежде всего хотелось бы избавиться от конструкции выбора в setArgument [G23]. В идеале она должна быть заменена единственным вызовом ArgumentMarshaler.set. Это означает, что код setIntArg, setStringArg и setBooleanArg должен быть перемещен в соответствующие классы, производные от ArgumentMarshaler. Однако при этом возникает одна проблема.

Внимательно присмотревшись к функции setIntArg, можно заметить, что в ней используются две переменные экземпляров: args и currentArg. Чтобы переместить setIntArg в BooleanArgumentMarshaler, мне придется передать args и currentArgs в аргументах при вызове. Решение получается «грязным» [F1]. Я бы предпочел передать один аргумент вместо двух. К счастью, у проблемы существует простое решение: мы можем преобразовать массив args в list и передать Iterator функциям set. Следующее преобразование было проведено за десять шагов, с обязательным выполнением всех тестов после каждого шага. Здесь я приведу только конечный результат, но вы легко сможете опознать большинство промежуточных шагов по этому листингу.

public class Args {
  private String schema;
  private String[] args;
  private boolean valid = true;
  private Set unexpectedArguments = new TreeSet();
  private Map marshalers =
new HashMap();
  private Set argsFound = new HashSet();
  private Iterator currentArgument;
  private char errorArgumentId = '\0';
  private String errorParameter = "TILT";
  private ErrorCode errorCode = ErrorCode.OK;
private List argsList;

  private enum ErrorCode {
    OK, MISSING_STRING, MISSING_INTEGER, INVALID_INTEGER, UNEXPECTED_ARGUMENT}

  public Args(String schema, String[] args) throws ParseException {
    this.schema = schema;
    argsList = Arrays.asList(args);
    valid = parse();
  }

  private boolean parse() throws ParseException {
    if (schema.length() == 0 && argsList.size() == 0)
      return true;
    parseSchema();
    try {
      parseArguments();
    } catch (ArgsException e) {
    }
    return valid;
  }
---
  private boolean parseArguments() throws ArgsException {
    for (currentArgument = argsList.iterator(); currentArgument.hasNext();) {
      String arg = currentArgument.next();
      parseArgument(arg);
    }
    return true;
  }
---
  private void setIntArg(ArgumentMarshaler m) throws ArgsException {
    String parameter = null;
    try {
      parameter = currentArgument.next();
      m.set(parameter);
    } catch (NoSuchElementException 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 {
    try {
      m.set(currentArgument.next());
    } catch (NoSuchElementException e) {
      errorCode = ErrorCode.MISSING_STRING;
      throw new ArgsException();
    }
  }

Все изменения были простыми и не нарушали работы тестов. Теперь можно заняться перемещением функций в соответствующие производные классы. Начнем с внесения изменений в setArgument:

private boolean setArgument(char argChar) throws ArgsException {
  ArgumentMarshaler m = marshalers.get(argChar);
  if (m == null)
    return false;
  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;
}

Это изменение важно, потому что мы хотим полностью устранить цепочку if-else. Для этого из нее необходимо вывести состояние ошибки.

Теперь можно переходить к перемещению функций set. Функция setBooleanArg тривиальна, поэтому начнем с нее. Наша задача — изменить функцию setBooleanArg так, чтобы она просто передавала управление BooleanArgumentMarshaler.

private boolean setArgument(char argChar) throws ArgsException {
    ArgumentMarshaler m = marshalers.get(argChar);
    if (m == null)
      return false;
    try {
      if (m instanceof BooleanArgumentMarshaler)
        setBooleanArg(m, currentArgument);
      else if (m instanceof StringArgumentMarshaler)
        setStringArg(m);
      else if (m instanceof IntegerArgumentMarshaler)
        setIntArg(m);