Java with sonar: reduce complexity of method

Question

public Meteo getMeteoTorneo() {
    
    Meteo meteo = new Meteo(cittaString);
    
    Document doc = null;
    try {
        doc = Jsoup.connect("https://www.ilmeteo.it/meteo/" + torneo.getCitta()).get();
    } catch (IOException e) {
        e.printStackTrace();
    }
    Elements newsHeadlines = doc.getElementsByTag("li");
    for (Element headline : newsHeadlines) {
        if (headline.text().split(" ").length > 2) {
            String tmax = headline.getElementsByClass("tmax").text();
            String tmin = headline.getElementsByClass("tmin").text();
            String giorno = headline.getElementsByTag("span").first() != null ? headline.getElementsByTag("span").first().text() : "";
            boolean rain  = headline.getElementsByClass("s flag_pioggia").isEmpty();
            boolean nuvoloso  = headline.getElementsByClass("s ss3").isEmpty();
            boolean sole  = headline.getElementsByClass("s ss1").isEmpty();
            if(controllaTemp(tmin, tmax))
                continue;

            if((controllaData() && (giorno.split(" ")[1].equals(""+torneo.getData().getDayOfMonth())))){
                    if(!rain)
                        meteo.setT("1");
                    else if(!nuvoloso)
                        meteo.setT("2");
                    else if(!sole)
                        meteo.setT("3");
                    meteo.settMin(tmin);
                    meteo.settMax(tmax);
                    
                }
            
        }
    }
    return meteo;

}

Hi, I have a problem with this method. Sonar tells me to reduce its Cognitive Complexity from 21 to the 15 allowed, but I have no idea what I have to do, because I can remove nothing. Could you suggest something? Thanks

Refactor. it is not about "removing" things, but restructuring things. For example, the outermost `if` looks like it could be moved in a separate method. — Turing85, Jun 01 '21 at 15:00
Yeah, it's better to quit early than write nested branches - discard cases first, then proceed with code as normal. It's called short-circuiting. This is not an error, it's a warning - sometimes warnings are crap. But I generally advise from what you do in line with assigning `giorno` - you evaluate things twice. Do yourself a favor and use variables... — Tooster, Jun 01 '21 at 15:06
Take a look at this answer https://stackoverflow.com/a/66937330/5932611 — Tharindu Sathischandra, Jun 01 '21 at 17:04

score 1 · Accepted Answer · answered Jun 01 '21 at 15:12

You don't need to remove anything, you can extract the information inside of the "for" in a method, like this:

public Meteo getMeteoTorneo() {

    Meteo meteo = new Meteo(cittaString);

    Document doc = null;
    try {
        doc = Jsoup.connect("https://www.ilmeteo.it/meteo/" + torneo.getCitta()).get();
    } catch (IOException e) {
        e.printStackTrace();
    }
    Elements newsHeadlines = doc.getElementsByTag("li");
    for (Element headline : newsHeadlines) {
        if (headline.text().split(" ").length > 2) {
            updateMeteo(headline, meteo)
        }
    }
    return meteo;
}

private void updateMeteo(Element headline, Meteo meteo) {
    String tmax = headline.getElementsByClass("tmax").text();
    String tmin = headline.getElementsByClass("tmin").text();
    String giorno = headline.getElementsByTag("span").first() != null ? headline.getElementsByTag("span").first().text() : "";
    boolean rain  = headline.getElementsByClass("s flag_pioggia").isEmpty();
    boolean nuvoloso  = headline.getElementsByClass("s ss3").isEmpty();
    boolean sole  = headline.getElementsByClass("s ss1").isEmpty();
    if(controllaTemp(tmin, tmax))
        return;

    if((controllaData() && (giorno.split(" ")[1].equals(""+torneo.getData().getDayOfMonth())))){
        if(!rain)
            meteo.setT("1");
        else if(!nuvoloso)
            meteo.setT("2");
        else if(!sole)
            meteo.setT("3");
            meteo.settMin(tmin);
            meteo.settMax(tmax);          
    }
}

Now if you look at it, the meteo object would always set the same variables (T, Min and Max). So probably is better to get those three elements and set them at the end (using a Tuple3 or, even better, your own class)

Java with sonar: reduce complexity of method

1 Answers1