Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have a ContratMercan with 2 Timestamps containing start and end dates. I have to create a new entity SeguimientoSeccion for each month between this 2 dates.

I have 2 questions:

  • There is a way to improve it?
  • After this I will need to manipulate the List<SeguimientoSeccion> secciones to see which one matches other entity... I though about making a Map but there can be 2 or more sections in each month and year

private List<SeguimientoSeccion> createSectionsPerMonth(ContratMercan contrato) 
{
    List<SeguimientoSeccion> secciones = new ArrayList<SeguimientoSeccion>();

    Calendar start = Calendar.getInstance();
    start.setTime(contrato.getPeriodoEntregaIni());
    start.set(Calendar.DAY_OF_MONTH, 1);

    Calendar end = Calendar.getInstance();
    end.setTime(contrato.getPeriodoEntregaFin());
    end.set(Calendar.DAY_OF_MONTH, 2);  // for loop comparison

    for (int month = start.get(Calendar.MONTH), year = start.get(Calendar.YEAR); start.before(end); month = start.get(Calendar.MONTH), year = start.get(Calendar.YEAR)) {
        SeguimientoSeccion seccion = new SeguimientoSeccion();
        seccion.setMonth(month + 1);  // natural month number
        seccion.setYear(year);

        start.add(Calendar.MONTH, 1);

        secciones.add(seccion);
    }

    return secciones;
}
share|improve this question

The code would be simpler if you used a Calendar object as the loop variable.

Calendar current = …;
Calendar end = …;

for (; current.before(end); current.add(Calendar.MONTH, 1)) {
    SeguimientoSeccion seccion = new SeguimientoSeccion();
    seccion.setMonth(1 + current.get(Calendar.MONTH));  // natural month number
    seccion.setYear(current.get(Calendar.YEAR));
    secciones.add(seccion);
}

Consider implementing a SeguimientoSeccion.setMonthYear(m, y) method instead, as it's probably useless to set one of the fields without also setting the other.

share|improve this answer
    
Already implemented, works and looks great.:). Any idea about how to make a Collection or Map to refer the objects later? – Jordi Castilla Nov 10 '15 at 10:40

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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