2
\$\begingroup\$

I have an entity with opening and closing times and I have to list timetable in HTML label.

  • Times can be empty.
  • Ugly attribute names getHorarioMIni or getHorarioTFin cannot be changed due external DB relations and the auto-populating methods.

How to optimize this function:

public String getTimetable() {
    final String SEPARATOR = " - "; 
    final String NEW_LINE = "<br>"; 
    StringBuilder sb  = new StringBuilder();

    if (nNoE(getHorarioMIni())) 
        sb.append(getHorarioMIni());
    if (nNoE(getHorarioMIni()) && nNoE(getHorarioMFin()))
        sb.append(SEPARATOR);
    if (nNoE(getHorarioMFin()))
        sb.append(getHorarioMFin());

    if ((nNoE(getHorarioMIni()) || nNoE(getHorarioMFin())) && (nNoE(getHorarioTIni()) || nNoE(getHorarioTFin())))
        sb.append(NEW_LINE); 

    if (nNoE(getHorarioTIni())) 
        sb.append(getHorarioTIni());
    if (nNoE(getHorarioTIni()) && nNoE(getHorarioTFin()))
        sb.append(SEPARATOR);
    if (nNoE(getHorarioTFin()))
        sb.append(getHorarioTFin());

    System.out.println(sb);
    return sb.toString();
}

private boolean nNoE(String toCheck) {
    return  toCheck != null && !toCheck.isEmpty();
}

Expected outputs (just commenting example constructor attributes):

08:00 - 12:00<br>15:00 - 18:00
15:00 - 18:00
08:00 - 12:00
08:00 - 12:00<br>18:00

Extra There is an easy way to achieve this? if only one of each group (T and M) filled?

08:00 - 18:00

MCVE

To test it you can copy paste in a java class named Main and execute

class Main {
    public static void main(String args[]) throws Exception {
        Silo silo = new Silo();
        System.out.println(silo.getHorarios());
    }
}


class Silo {

    public Silo() {
        // comment any time to check different outputs.
        this.horarioMIni = "8:00";
        this.horarioMFin = "12:00";

        this.horarioTIni = "15:00";
        this.horarioTFin = "18:00";
    }

    // M group (morning)    
    private String horarioMIni;
    private String horarioMFin;
    // T group (evening)    
    private String horarioTIni;
    private String horarioTFin;

    public String getHorarioMIni() {
        return horarioMIni;
    }
    public void setHorarioMIni(String horarioMIni) {
        this.horarioMIni = horarioMIni;
    }
    public String getHorarioMFin() {
        return horarioMFin;
    }
    public void setHorarioMFin(String horarioMFin) {
        this.horarioMFin = horarioMFin;
    }
    public String getHorarioTIni() {
        return horarioTIni;
    }
    public void setHorarioTIni(String horarioTIni) {
        this.horarioTIni = horarioTIni;
    }
    public String getHorarioTFin() {
        return horarioTFin;
    }
    public void setHorarioTFin(String horarioTFin) {
        this.horarioTFin = horarioTFin;
    }
    public String getHorarios() {
        final String SEPARATOR = " - "; 
        final String NEW_LINE = "<br>"; 
        StringBuilder sb  = new StringBuilder();

        if (nNoE(getHorarioMIni())) 
            sb.append(getHorarioMIni());
        if (nNoE(getHorarioMIni()) && nNoE(getHorarioMFin()))
            sb.append(SEPARATOR);
        if (nNoE(getHorarioMFin()))
            sb.append(getHorarioMFin());

        if ((nNoE(getHorarioMIni()) || nNoE(getHorarioMFin())) && (nNoE(getHorarioTIni()) || nNoE(getHorarioTFin())))
            sb.append(NEW_LINE); 

        if (nNoE(getHorarioTIni())) 
            sb.append(getHorarioTIni());
        if (nNoE(getHorarioTIni()) && nNoE(getHorarioTFin()))
            sb.append(SEPARATOR);
        if (nNoE(getHorarioTFin()))
            sb.append(getHorarioTFin());

        System.out.println(sb);
        return sb.toString();
    }

    private boolean nNoE(String toCheck) {
        return  toCheck != null && !toCheck.isEmpty();
    }

}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Working around 'ugly' method names

Ok, so you realized you are constrained by the getter names due to some 'configuration-by-convention' features of your application/framework. If you think this can be fixed permanently in the future, and require a short-term workaround, you can consider having wrapper methods to... remind you to have the permanent fix sooner than later?

public String getHorarioTIni() {
    return getEveningStartTime();
}

// use this instead
public String getEveningStartTime() {
    // here's hoping you get to rename the field names too
    return horarioTIni;
}

Also, you mentioned the getters only, so I guess there's no excuse for nNoE(). :) Reading it as isNotNullAndEmpty() is easier on the brains.

Avoiding code repetition

If you take a closer look at what you're doing now, it really boils down to these steps:

  1. Define a 'duration description' (DD) as a start time, an optional separator, and an end time. The separator is required when both times are present.
  2. Construct a DD for the AM session.
  3. If there are DDs for both AM and PM sessions, insert the newline.
  4. Construct a DD for the PM session.
  5. Concatenate the DDs for both sessions with the optional newline in between.

Therefore, you can extract step 1 as a method:

// this can be made a class field
private static final String SEPARATOR = " - "; 

private static String createDescription(String startTime, String endTime) {
    StringBuilder builder = new StringBuilder(Objects.toString(startTime, ""));
    if (isNotNullAndEmpty(startTime) && isNotNullAndEmpty(endTime)) {
        builder.append(SEPARATOR);
    }
    return builder.append(Objects.toString(endTime, "")).toString();
}

Objects.toString(Object, String) is used here to make the String generation slightly more expressive.

The method body is now more compact, and hopefully more readable as a result:

public String getTimetable() {
    final String NEW_LINE = "<br>"; 
    String morningDuration = createDescription(getMorningStartTime(), getMorningEndTime());
    String eveningDuration = createDescription(getEveningStartTime(), getEveningEndTime());
    StringBuilder builder = new StringBuilder(morningDuration);
    if (!morningDuration.isEmpty() && !eveningDuration.isEmpty()) {
        builder.append(NEW_LINE); 
    }
    return builder.append(eveningDuration).toString();
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.