Избегайте слишком сложного метода - цикломатическая сложность

23

Не уверен, как поступить с этим методом, чтобы уменьшить цикломатическую сложность. Сонар сообщает 13, тогда как 10 ожидается. Я уверен, что нет ничего плохого в том, чтобы оставить этот метод, поскольку он просто бросает мне вызов, как поступить в соответствии с правилом Сонара. Любые мысли будут с благодарностью.

 public static long parseTimeValue(String sValue) {

    if (sValue == null) {
        return 0;
    }

    try {
        long millis;
        if (sValue.endsWith("S")) {
            millis = new ExtractSecond(sValue).invoke();
        } else if (sValue.endsWith("ms")) {
            millis = new ExtractMillisecond(sValue).invoke();
        } else if (sValue.endsWith("s")) {
            millis = new ExtractInSecond(sValue).invoke();
        } else if (sValue.endsWith("m")) {
            millis = new ExtractInMinute(sValue).invoke();
        } else if (sValue.endsWith("H") || sValue.endsWith("h")) {
            millis = new ExtractHour(sValue).invoke();
        } else if (sValue.endsWith("d")) {
            millis = new ExtractDay(sValue).invoke();
        } else if (sValue.endsWith("w")) {
            millis = new ExtractWeek(sValue).invoke();
        } else {
            millis = Long.parseLong(sValue);
        }

        return millis;

    } catch (NumberFormatException e) {
        LOGGER.warn("Number format exception", e);
    }

    return 0;
}

Все методы ExtractXXX определены как staticвнутренние классы. Например, как один ниже -

    private static class ExtractHour {
      private String sValue;

      public ExtractHour(String sValue) {
         this.sValue = sValue;
      }

      public long invoke() {
         long millis;
         millis = (long) (Double.parseDouble(sValue.substring(0, sValue.length() - 1)) * 60 * 60 * 1000);
         return millis;
     }
 }

ОБНОВЛЕНИЕ 1

Я собираюсь успокоиться со смесью предложений здесь, чтобы удовлетворить парня Сонара. Определенно место для улучшений и упрощений.

Гуава Function- просто нежелательная церемония здесь. Хотел обновить вопрос о текущем статусе. Здесь нет ничего окончательного. Пожалуйста, поделитесь своими мыслями

public class DurationParse {

private static final Logger LOGGER = LoggerFactory.getLogger(DurationParse.class);
private static final Map<String, Function<String, Long>> MULTIPLIERS;
private static final Pattern STRING_REGEX = Pattern.compile("^(\\d+)\\s*(\\w+)");

static {

    MULTIPLIERS = new HashMap<>(7);

    MULTIPLIERS.put("S", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractSecond(input).invoke();
        }
    });

    MULTIPLIERS.put("s", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractInSecond(input).invoke();
        }
    });

    MULTIPLIERS.put("ms", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractMillisecond(input).invoke();
        }
    });

    MULTIPLIERS.put("m", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractInMinute(input).invoke();
        }
    });

    MULTIPLIERS.put("H", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractHour(input).invoke();
        }
    });

    MULTIPLIERS.put("d", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractDay(input).invoke();
        }
    });

    MULTIPLIERS.put("w", new Function<String, Long>() {
        @Nullable
        @Override
        public Long apply(@Nullable String input) {
            return new ExtractWeek(input).invoke();
        }
    });

}

public static long parseTimeValue(String sValue) {

    if (isNullOrEmpty(sValue)) {
        return 0;
    }

    Matcher matcher = STRING_REGEX.matcher(sValue.trim());

    if (!matcher.matches()) {
        LOGGER.warn(String.format("%s is invalid duration, assuming 0ms", sValue));
        return 0;
    }

    if (MULTIPLIERS.get(matcher.group(2)) == null) {
        LOGGER.warn(String.format("%s is invalid configuration, assuming 0ms", sValue));
        return 0;
    }

    return MULTIPLIERS.get(matcher.group(2)).apply(matcher.group(1));
}

private static class ExtractSecond {
    private String sValue;

    public ExtractSecond(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = Long.parseLong(sValue);
        return millis;
    }
}

private static class ExtractMillisecond {
    private String sValue;

    public ExtractMillisecond(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue));
        return millis;
    }
}

private static class ExtractInSecond {
    private String sValue;

    public ExtractInSecond(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue) * 1000);
        return millis;
    }
}

private static class ExtractInMinute {
    private String sValue;

    public ExtractInMinute(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue) * 60 * 1000);
        return millis;
    }
}

private static class ExtractHour {
    private String sValue;

    public ExtractHour(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue) * 60 * 60 * 1000);
        return millis;
    }
}

private static class ExtractDay {
    private String sValue;

    public ExtractDay(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue) * 24 * 60 * 60 * 1000);
        return millis;
    }
}

private static class ExtractWeek {
    private String sValue;

    public ExtractWeek(String sValue) {
        this.sValue = sValue;
    }

    public long invoke() {
        long millis;
        millis = (long) (Double.parseDouble(sValue) * 7 * 24 * 60 * 60 * 1000);
        return millis;
    }
}

}


ОБНОВЛЕНИЕ 2

Хотя я добавил свое обновление, оно того стоит. Я собираюсь идти дальше, так как Сонар сейчас не жалуется. Не волнуйтесь, я принимаю ответ mattnz, так как это путь, и я не хочу показывать плохой пример тем, кто сталкивается с этим вопросом. Итог - Не переусердствуйте, потому что Сонар (или руководитель проекта Half Baked) жалуется на CC. Просто сделайте то, что стоит копейки для проекта. Спасибо всем.

asyncwait
источник
4
Самый простой ответ на этот вопрос: private static Dictionary<string,Func<string,long>> _mappingStringToParser;я оставлю все остальное как упражнение для вас (или человека, у которого сейчас больше свободного времени, чем у меня). Если вы знакомы с монадическими парсерами, вы можете найти более чистый API, но я не пойду туда прямо сейчас
Джимми Хоффа,
Очень хотелось бы, чтобы вы могли сэкономить время на «монадических парсерах» и на том, как это можно применить к довольно маленькой функции, такой как эта. И этот кусок кода из Java.
asyncwait
как ExtractBlahопределяются классы? это из какой-то библиотеки или доморощенного?
комнат
4
Приложенный код ведет меня немного дальше к более простой реализации: ваша фактическая разница - это множитель. Создайте карту из них: вытащите альфа-символы из конца вашего sValue, используйте их в качестве ключа, а затем потяните все до альф с фронта для значения, которое вы умножаете на сопоставленный множитель.
Джимми Хоффа
2
Re Update: я единственный, кто имеет "Over Engineered" звонит в моей голове?
Mattnz

Ответы:

46

Ответ разработки программного обеспечения:

Это всего лишь один из многих случаев, когда простой подсчет бинов, которые просто подсчитать, заставит вас поступить неправильно. Это не сложная функция, не меняйте ее. Цикломатическая сложность - это всего лишь руководство к сложности, и вы плохо ее используете, если изменяете эту функцию на ее основе. Он прост, удобочитаем, удобен в обслуживании (на данный момент), если в будущем он станет больше, СС будет расти в геометрической прогрессии и получит то внимание, которое ему нужно, когда это нужно, а не раньше.

Миньон работает в крупной международной корпорации Ответ:

Организации полны переплаченных, непродуктивных команд счетчиков бобов. Удовлетворять счетчики бобов легче и, конечно, мудрее, чем делать правильные вещи. Вам нужно изменить порядок действий, чтобы снизить CC до 10, но будьте честны с тем, почему вы это делаете - чтобы не было счетчиков бобов за спиной. Как предлагается в комментариях - «монадные парсеры» могут помочь

mattnz
источник
14
+1 за уважение причины правила, а не самого правила
Раду Мурзе
5
Хорошо сказано! Однако в других случаях, когда у вас CC = 13 с несколькими уровнями вложенности и сложной (ветвящейся) логикой, было бы предпочтительнее, по крайней мере, попытаться упростить его.
Grizwako
16

Спасибо @JimmyHoffa, @MichaelT и @ GlenH7 за помощь!

питон

Перво-наперво, вы действительно должны принимать только известные префиксы, то есть «H» или «H». Если вам нужно принять и то и другое, вам следует выполнить некоторую операцию, чтобы согласованно сохранить место на вашей карте.

В Python вы можете создать словарь.

EXTRACTION_MAP = {
    'S': ExtractSecond,
    'ms': ExtractMillisecond,
    'm': ExtractMinute,
    'H': ExtractHour,
    'd': ExtractDay,
    'w': ExtractWeek
}

Тогда мы хотим, чтобы метод использовал это:

def parseTimeValue(sValue)
    ending = ''.join([i for i in sValue if not i.isdigit()])
    return EXTRACTION_MAP[ending](sValue).invoke()

Должен иметь лучшую цикломатическую сложность.


Джава

Нам нужен только 1 (один) каждого множителя. Давайте поместим их в карту, как предлагают некоторые другие ответы.

Map<String, Float> multipliers = new HashMap<String, Float>();
    map.put("S", 60 * 60);
    map.put("ms", 60 * 60 * 1000);
    map.put("m", 60);
    map.put("H", 1);
    map.put("d", 1.0 / 24);
    map.put("w", 1.0 / (24 * 7));

Тогда мы можем просто использовать карту, чтобы получить правильный конвертер

Pattern foo = Pattern.compile(".*(\\d+)\\s*(\\w+)");
Matcher bar = foo.matcher(sValue);
if(bar.matches()) {
    return (long) (Double.parseDouble(bar.group(1)) * multipliers.get(bar.group(2);
}
Ampt
источник
Да, сопоставление строк с коэффициентом преобразования было бы гораздо более простым решением. Если им не нужны объекты, то они должны избавиться от них, но я не могу видеть остальную часть программы, так что, может быть, они используются больше как объекты где-то еще ...?
FrustratedWithFormsDesigner
@FrustratedWithFormsDesigner они могут, но в рамках этого метода он просто возвращает long и экземпляр объекта выпадает из области видимости. Кроме того, это имеет побочный эффект: если этот код вызывается чаще, количество часто используемых короткоживущих объектов без состояния сокращается.
Ни один из этих ответов не может рассматриваться как обеспечивающий то же решение, что и исходная программа, поскольку они основаны на предположениях, которые могут быть недействительными. Java-код: как вы уверены, что единственное, что делают методы, это применяет множитель? Код Python: как вы уверены, что строка не может содержать начальные (или средние точки) символы, кроме цифр (например, «123.456s»).
Mattnz
@mattnz - взгляните на имена переменных в приведенных примерах. Понятно, что ОП получает единицу времени в виде строки, а затем необходимо преобразовать ее в единицу времени другого типа. Таким образом, примеры, приведенные в этом ответе, относятся непосредственно к домену ОП. Игнорируя этот аспект, ответ по-прежнему предоставляет общий подход, который можно использовать для другой области. Этот ответ решает проблему, которая была представлена, а не проблему, которая могла быть представлена.
5
@mattnz - 1) OP никогда не указывает это в своем примере и может не заботиться. Откуда вы знаете, что предположения неверны? 2) Общий метод все еще будет работать, потенциально требуя более сложного регулярного выражения. 3) Суть ответа заключается в том, чтобы предоставить концептуальный путь решения цикломатической сложности, а не обязательно конкретный, компилируемый ответ. 4) хотя этот ответ игнорирует более широкий аспект «имеет значение сложность», он косвенно отвечает на проблему, показывая альтернативную форму для кода.
3

Так как вы return millisв конце этого ужасного ifelseifelse в любом случае, первое, что приходит на ум, это немедленно вернуть значение изнутри блоков if. Этот подход следует за тем, который указан в каталоге шаблонов рефакторинга как Заменить Вложенное Условное с Охранными предложениями .

Метод имеет условное поведение, которое не дает понять, каков нормальный путь выполнения

Используйте пункты охраны для всех особых случаев

Это поможет вам избавиться от чужих, сгладить код и сделать сонар счастливым:

    if (sValue.endsWith("S")) {
        return new ExtractSecond(sValue).invoke();
    } // no need in else after return, code flattened

    if (sValue.endsWith("ms")) {
        return new ExtractMillisecond(sValue).invoke();
    }

    // and so on...
    return Long.parseLong(sValue); // forget millis, these aren't needed anymore

Еще одна вещь, которую стоит рассмотреть, это сбросить блок try-catch. Это также снизит цикломатическую сложность, но главная причина, по которой я рекомендую это, состоит в том, что в этом блоке нет никакого способа для кода вызывающего абонента отличить юридически проанализированный 0 от исключения числового формата.

Если вы не уверены на 200%, что возвращение 0 для ошибок синтаксического анализа - это то, что нужно коду вызывающей стороны, вам лучше распространить это исключение и позволить коду вызывающей стороны решить, как с ним работать. Как правило, в вызывающей программе более удобно решить, прервать ли выполнение или повторить попытку получения ввода, или вернуться к какому-либо значению по умолчанию, например 0 или -1, или как угодно.


Ваш фрагмент кода для примера ExtractHour заставляет меня чувствовать, что функциональность ExtractXXX разработана далеко не оптимально. Бьюсь об заклад, каждый из оставшихся классов бездумно повторяет одно parseDoubleи то же substring, и умножает такие вещи, как 60 и 1000 снова и снова и снова.

Это потому, что вы упустили суть того, что нужно сделать, в зависимости от того, что sValueименно: он определяет, сколько символов нужно вырезать из конца строки и каково будет значение множителя. Если вы спроектируете свой «основной» объект на основе этих важных функций, он будет выглядеть следующим образом:

private static class ParseHelper {
    // three things you need to know to parse:
    final String source;
    final int charsToCutAtEnd;
    final long multiplier;

    ParseHelper(String source, int charsToCutAtEnd, long multiplier) {
        this.source = source == null ? "0" : source; // let's handle null here
        this.charsToCutAtEnd = charsToCutAtEnd;
        this.multiplier = multiplier;
    }

    long invoke() {
        // NOTE consider Long.parseLong instead of Double.parseDouble here
        return (long) (Double.parseDouble(cutAtEnd()) * multiplier);
    }

    private String cutAtEnd() {
        if (charsToCutAtEnd == 0) {
            return source;
        }
        // write code that cuts 'charsToCutAtEnd' from the end of the 'source'
        throw new UnsupportedOperationException();
    }
}

После этого вам понадобится код, который настраивает вышеуказанные объекты для конкретного условия, если оно выполнено, или как-то «обходит» иное. Это можно сделать следующим образом:

private ParseHelper setupIfInSecond(ParseHelper original) {
    final String sValue = original.source;
    return sValue.endsWith("s") && !sValue.endsWith("ms")
            ? new ParseHelper(sValue, 1, 1000)
            :  original; // bypass
}

private ParseHelper setupIfMillisecond(ParseHelper original) {
    final String sValue = original.source;
    return sValue.endsWith("ms")
            ? new ParseHelper(sValue, 2, 1)
            : original; // bypass
}

// and so on...

Исходя из описанных выше строительных блоков , код вашего метода может выглядеть следующим образом:

public long parseTimeValue(String sValue) {

   return setupIfSecond(
           setupIfMillisecond(
           setupIfInSecond(
           setupIfInMinute(
           setupIfHour(
           setupIfDay(
           setupIfWeek(
           new ParseHelper(sValue, 0, 1))))))))
           .invoke();
}

Видите ли, в методе не осталось никаких сложностей, никаких фигурных скобок (ни многократных возвратов, как в моем первоначальном предложении грубой силы при выравнивании кода). Вы просто последовательно проверяете ввод и корректируете обработку по мере необходимости.

комар
источник
1

Если вы действительно хотите провести рефакторинг, вы можете сделать что-то вроде этого:

// All of your Extract... classes will have to implement this interface!
public Interface TimeExtractor
{
    public long invoke();
}

private static class ExtractHour implements TimeExtractor
{
  private String sValue;


  /*Not sure what this was for - might not be necessary now
  public ExtractHour(String sValue)
  {
     this.sValue = sValue;
  }*/

  public long invoke(String s)
  {
     this.sValue = s;
     long millis;
     millis = (long) (Double.parseDouble(sValue.substring(0, sValue.length() - 1)) * 60 * 60 * 1000);
     return millis;
 }
}

private static HashMap<String, TimeExtractor> extractorMap= new HashMap<String, TimeExtractor>();

private void someInitMethod()
{
   ExtractHour eh = new ExtractorHour;
   extractorMap.add("H",eh);
   /*repeat for all extractors */
}

public static long parseTimeValue(String sValue)
{
    if (sValue == null)
    {
        return 0;
    }
    String key = extractKeyFromSValue(sValue);
    long millis;
    TimeExtractor extractor = extractorMap.get(key);
    if (extractor!=null)
    {
      try
      {
         millis= extractor.invoke(sValue);
      }
        catch (NumberFormatException e)
      {
          LOGGER.warn("Number format exception", e);
      }
    }
    else
       LOGGER.error("NO EXTRACTOR FOUND FOR "+key+", with sValue: "+sValue);

    return millis;
}

Идея состоит в том, что у вас есть карта ключей (которую вы постоянно используете в "setswith"), которые сопоставляются с конкретными объектами, которые выполняют необходимую обработку.

Здесь немного грубо, но я надеюсь, что это достаточно ясно. Я не заполнил детали, extractKeyFromSValue()потому что я просто недостаточно знаю, что это за строки, чтобы сделать это правильно. Похоже, что это последние 1 или 2 нечисловых символа (регулярное выражение, вероятно, может извлечь его достаточно легко, возможно, .*([a-zA-Z]{1,2})$будет работать), но я не уверен на 100% ...


Оригинальный ответ:

Вы могли бы изменить

else if (sValue.endsWith("H") || sValue.endsWith("h")) {

в

else if (sValue.toUpper().endsWith("H")) {

Это может немного вас спасти, но, честно говоря, я бы не слишком волновался об этом. Я согласен с вами, что я не думаю, что оставить метод таким, какой он есть, приносит много вреда. Вместо того, чтобы пытаться «подчиняться правилам Сонара», старайтесь «держаться как можно ближе к правилам Сонара».

Вы можете свести себя с ума, пытаясь следовать каждому отдельному правилу, которое будут иметь эти инструменты анализа, но вы также должны решить, имеют ли правила смысл для вашего проекта, и для конкретных случаев, когда время, потраченное на рефакторинг, может не стоить того ,

FrustratedWithFormsDesigner
источник
3
Не очень полезен, сонар до сих пор жалуется. Я как бы пытаюсь это для удовольствия, по крайней мере, позволяет учиться один или два. Код перенесен в постановку, хотя.
asyncwait
@asyncwait: Ах, я думал, что вы восприняли этот доклад от сонара более серьезно. Да, изменения, которые я предложил, не будут иметь большого значения - я не думаю, что это займет у вас от 13 до 10, но в некоторых инструментах, которые я видел, подобные вещи имеют несколько заметную разницу.
FrustratedWithFormsDesigner
Просто добавление еще одной ветки IF увеличило CC до +1.
asyncwait
0

Вы можете рассмотреть возможность использования enum для хранения всех доступных вариантов и предикатов для сопоставления значений. Как упоминалось ранее, ваша функция достаточно читабельна, чтобы оставить ее неизменной. Эти показатели помогут вам, а не наоборот.

//utility class for matching values
private static class ValueMatchingPredicate implements Predicate<String>{
    private final String[] suffixes;

    public ValueMatchingPredicate(String[] suffixes) {      
        this.suffixes = suffixes;
    }

    public boolean apply(String sValue) {
        if(sValue == null) return false;

        for (String suffix : suffixes) {
            if(sValue.endsWith(suffix)) return true;
        }

        return false;
    }

    public static Predicate<String> withSuffix(String... suffixes){         
        return new ValueMatchingPredicate(suffixes);
    }       
}

//enum containing all possible options
private static enum TimeValueExtractor {                
    SECOND(
        ValueMatchingPredicate.withSuffix("S"), 
        new Function<String, Long>(){ 
            public Long apply(String sValue) {  return new ExtractSecond(sValue).invoke(); }
        }),

    MILISECOND(
        ValueMatchingPredicate.withSuffix("ms"), 
        new Function<String, Long>(){
            public Long apply(String sValue) { return new ExtractMillisecond(sValue).invoke(); }
        }),

    IN_SECOND(
        ValueMatchingPredicate.withSuffix("s"),
        new Function<String, Long>(){
            public Long apply(String sValue) { return new ExtractInSecond(sValue).invoke(); }
        }),

    IN_MINUTE(
        ValueMatchingPredicate.withSuffix("m"),
        new Function<String, Long>(){
            public Long apply(String sValue) {  return new ExtractInMinute(sValue).invoke(); }
        }),

    HOUR(
        ValueMatchingPredicate.withSuffix("H", "h"),
        new Function<String, Long>(){
            public Long apply(String sValue) {  return new ExtractHour(sValue).invoke(); }
        }),

    DAY(
        ValueMatchingPredicate.withSuffix("d"),
        new Function<String, Long>(){
            public Long apply(String sValue) {  return new ExtractDay(sValue).invoke(); }
        }),

    WEEK(
        ValueMatchingPredicate.withSuffix("w"),
        new Function<String, Long>(){
            public Long apply(String sValue) {  return new ExtractWeek(sValue).invoke(); }
        });

    private final Predicate<String>      valueMatchingRule;
    private final Function<String, Long> extractorFunction;

    public static Long DEFAULT_VALUE = 0L;

    private TimeValueExtractor(Predicate<String> valueMatchingRule, Function<String, Long> extractorFunction) {
        this.valueMatchingRule = valueMatchingRule;
        this.extractorFunction = extractorFunction;
    }

    public boolean matchesValueSuffix(String sValue){
        return this.valueMatchingRule.apply(sValue);
    }

    public Long extractTimeValue(String sValue){
        return this.extractorFunction.apply(sValue);
    }

    public static Long extract(String sValue) throws NumberFormatException{
        TimeValueExtractor[] extractors = TimeValueExtractor.values();

        for (TimeValueExtractor timeValueExtractor : extractors) {
            if(timeValueExtractor.matchesValueSuffix(sValue)){
                return timeValueExtractor.extractTimeValue(sValue);
            }
        }

        return DEFAULT_VALUE;
    }
}

//your function
public static long parseTimeValue(String sValue){
    try{
        return TimeValueExtractor.extract(sValue);
    } catch (NumberFormatException e) {
        //LOGGER.warn("Number format exception", e);
        return TimeValueExtractor.DEFAULT_VALUE;
    }
}
Gwozdziu
источник
0

Связанный с вашим комментарием:

Итог - Не переусердствуйте, потому что Сонар (или руководитель проекта Half Baked) жалуется на CC. Просто сделайте то, что стоит копейки для проекта.

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

Но изменение стандартов команды в ответ на ситуации, которые не имеют смысла, является признаком хорошей команды с правильным отношением к стандартам. Стандарты призваны помочь команде, а не мешать написанию кода.


источник
0

Честно говоря, все технические ответы, представленные выше, кажутся ужасно сложными для поставленной задачи. Как уже было написано, сам код чист и хорош, поэтому я бы выбрал наименьшее возможное изменение, чтобы удовлетворить счетчик сложности. Как насчет следующего рефакторинга:

public static long parseTimeValue(String sValue) {

    if (sValue == null) {
        return 0;
    }

    try {
        return getMillis(sValue);
    } catch (NumberFormatException e) {
        LOGGER.warn("Number format exception", e);
    }

    return 0;
}

private static long getMillis(String sValue) {
    if (sValue.endsWith("S")) {
        return new ExtractSecond(sValue).invoke();
    } else if (sValue.endsWith("ms")) {
        return new ExtractMillisecond(sValue).invoke();
    } else if (sValue.endsWith("s")) {
        return new ExtractInSecond(sValue).invoke();
    } else if (sValue.endsWith("m")) {
        return new ExtractInMinute(sValue).invoke();
    } else if (sValue.endsWith("H") || sValue.endsWith("h")) {
        return new ExtractHour(sValue).invoke();
    } else if (sValue.endsWith("d")) {
        return new ExtractDay(sValue).invoke();
    } else if (sValue.endsWith("w")) {
        return new ExtractWeek(sValue).invoke();
    } else {
        return Long.parseLong(sValue);
    }
}

Если я считаю правильно, извлеченная функция должна иметь сложность 9, которая все еще соответствует требованиям. И это в основном тот же код, что и раньше , и это хорошо, так как код был хорош для начала.

Кроме того, читателям Чистого кода может понравиться тот факт, что метод верхнего уровня теперь прост и короток, а извлеченный - с деталями.

Arides
источник