Evitar método muito complexo - Complexidade Ciclomática

Não sei como seguir esse método para reduzir a complexidade ciclomática. O Sonar relata 13, enquanto 10 é esperado. Tenho certeza de que não há mal algum em deixar esse método, pois ele está apenas me desafiando a como obedecer às regras do Sonar. Qualquer pensamento seria muito apreciado.

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

Todos os métodos ExtractXXX são definidos como staticclasses internas. Por exemplo, como um abaixo -

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

ATUALIZAÇÃO 1

Vou me acalmar com uma mistura de sugestões aqui para satisfazer o cara do Sonar. Definitivamente espaço para melhorias e simplificação.

A goiaba Functioné apenas uma cerimônia indesejada aqui. Queria atualizar a pergunta sobre o status atual. Nada é final aqui. Despeje seus pensamentos por favor ..

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

}

ATUALIZAÇÃO 2

Embora eu tenha adicionado minha atualização, vale muito a pena. Vou seguir em frente, já que o Sonar agora não reclama. Não se preocupe muito e estou aceitando a resposta mattnz, pois é o caminho a percorrer e não quero dar um mau exemplo para quem se deparar com essa pergunta. Conclusão - Não exagere no engenheiro em nome do Sonar (ou gerente de projetos meio assados) queixa-se do CC. Basta fazer o que vale um centavo para o projeto. Obrigado a todos.

Resposta de engenharia de software:

Este é apenas um dos muitos casos em que simplesmente contar feijões simples de contar fará com que você faça a coisa errada. Não é uma função complexa, não mude. A complexidade ciclomática é apenas um guia para a complexidade e você a está usando mal se alterar essa função com base nela. É simples, legível, sustentável (por enquanto), se aumentar no futuro, o CC disparará exponencialmente e receberá a atenção de que precisa quando precisa, não antes.

Minion trabalhando para uma grande corporação multinacional Resposta:

As organizações estão cheias de equipes improdutivas e mal pagas de contadores de feijão. Manter os contadores de feijão felizes é mais fácil e certamente mais sábio do que fazer a coisa certa. Você precisa alterar a rotina para reduzir o CC para 10, mas seja honesto sobre o motivo de fazê-lo - para manter os contadores de feijão longe de suas costas. Conforme sugerido nos comentários - "analisadores monádicos" podem ajudar

Obrigado a @JimmyHoffa, @MichaelT e @ GlenH7 pela ajuda!

Python

Primeiramente, você deve realmente aceitar apenas prefixos conhecidos, ou seja, 'H' ou 'h'. Se você tiver que aceitar os dois, deverá executar alguma operação para torná-lo consistente para economizar espaço no seu mapa.

Em python, você pode criar um dicionário.

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

Então, queremos que o método use isso:

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

Deve ter uma melhor complexidade ciclomática.

Java

Precisamos apenas de 1 (um) de cada multiplicador. Vamos colocá-los em um mapa, como sugerem outras respostas.

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

Então podemos apenas usar o mapa para pegar o conversor certo

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

Uma vez que você, return millisno final daquele terrível caso contrário , de qualquer forma, a primeira coisa que vem à mente é retornar o valor imediatamente de dentro dos blocos if. Essa abordagem segue uma que está listada no catálogo de padrões de refatoração como Substituir condicional aninhado por cláusulas de guarda .

Um método tem comportamento condicional que não deixa claro qual é o caminho normal de execução.

Use cláusulas de guarda para todos os casos especiais

Isso o ajudará a se livrar dos outros, achatar o código e fazer o Sonar feliz:

    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

Outra coisa que vale a pena considerar é abandonar o bloco try-catch. Isso também reduzirá a complexidade ciclomática, mas o principal motivo pelo qual eu recomendo é que, com este bloco, não há como o código de chamada distinguir 0 analisado legalmente da exceção de formato numérico.

A menos que você tenha 200% de certeza de que é necessário o código 0 de retorno para erros de análise, é melhor propagar essa exceção e deixar que o código de chamada decida como lidar com ela. Normalmente, é mais conveniente decidir no chamador se deve interromper a execução ou tentar obter novamente a entrada ou retornar a algum valor padrão como 0 ou -1 ou qualquer outra coisa.

Seu snippet de código para um exemplo ExtractHour me faz sentir que a funcionalidade ExtractXXX foi projetada de maneira longe do ideal. Aposto que cada uma das classes restantes thoughtlessly repete mesma parseDoublee substring, e multiplicando o material como 60 e 1000, uma e outra e outra vez.

Isso ocorre porque você perdeu a essência do que precisa ser feito, dependendo sValue- a saber, define quantos caracteres cortar do final da string e qual seria o valor multiplicador. Se você projetar seu objeto "principal" em torno desses recursos essenciais, terá a seguinte aparência:

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

Depois disso, você precisará de um código que configure os objetos acima por condição específica se for atendido ou, de alguma forma, "ignora" o contrário. Isso pode ser feito da seguinte maneira:

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...

Com base nos blocos de construção acima , o código do seu método pode ter a seguinte aparência:

public long parseTimeValue(String sValue) {

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

Veja bem, não resta mais complexidade, nem chaves dentro do método (nem retornos múltiplos, como na minha sugestão de força bruta original no código de nivelamento). Você apenas verifica sequencialmente a entrada e ajusta o processamento conforme necessário.

Se você realmente deseja refatorá-lo, pode fazer algo assim:

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

A idéia é que você tenha um mapa de chaves (o que você está usando em "termina com" o tempo todo)) que mapeia para objetos específicos que fazem o processamento que você deseja.

É um pouco difícil aqui, mas espero que seja claro o suficiente. Não preenchi os detalhes extractKeyFromSValue()porque simplesmente não sei o suficiente sobre o que essas strings são para fazê-lo corretamente. Parece que são os últimos 1 ou 2 caracteres não numéricos (um regex provavelmente poderia extraí-lo com bastante facilidade, talvez .*([a-zA-Z]{1,2})$funcionasse), mas não tenho 100% de certeza ...

Resposta original:

Você pode mudar

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

para

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

Isso pode te poupar um pouco, mas sinceramente, eu não me preocuparia muito com isso. Concordo com você que não acho que haja muito mal em deixar o método como ele é. Em vez de tentar "obedecer às regras do Sonar", tente "ficar perto das diretrizes do Sonar, tanto quanto for razoavelmente possível".

Você pode enlouquecer tentando seguir todas as regras que essas ferramentas de análise terão nelas, mas também precisa decidir se as regras fazem sentido para o seu projeto e para casos específicos em que o tempo gasto na refatoração pode não valer a pena. .

Você pode considerar o uso de uma enumeração para armazenar todos os casos e predicados disponíveis para valores correspondentes. Como mencionado anteriormente, sua função é legível o suficiente apenas para mantê-la inalterada. Essas métricas existem para ajudá-lo a não ser o contrário.

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

Relacionado ao seu comentário de:

Conclusão - Não exagere no engenheiro em nome do Sonar (ou gerente de projetos meio assados) queixa-se do CC. Basta fazer o que vale um centavo para o projeto.

Outra opção a considerar é alterar os padrões de codificação de sua equipe para situações como esta. Talvez você possa adicionar algum tipo de votação em equipe para fornecer uma medida de governança e evitar situações de atalho.

Mas alterar os padrões da equipe em resposta a situações que não fazem sentido é um sinal de uma boa equipe com a atitude certa sobre os padrões. Os padrões existem para ajudar a equipe, não atrapalhar a escrita do código.

Para ser sincero, todas as respostas técnicas acima parecem terrivelmente complicadas para a tarefa em questão. Como já foi escrito, o código em si é limpo e bom, então eu optaria pela menor alteração possível para satisfazer o contador de complexidade. Que tal o seguinte refatorador:

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

Se eu estiver contando corretamente, a função extraída deve ter uma complexidade de 9, que ainda atende aos requisitos. E é basicamente o mesmo código de antes , o que é uma coisa boa, já que o código era bom para começar.

Além disso, os leitores do Clean Code podem aproveitar o fato de que o método de nível superior agora é simples e curto, enquanto o extraído lida com detalhes.