Code Kata - Berlin Clock To improve my programming style, I have been writing solutions for popular code katas. I have written a partial solution for the Berlin Clock kata and have been trying to follow the SOLID Principles. Can anyone critique the code and suggest improvements?

public class BerlinClock {

    private static List&lt;TimeUnit&gt; timeUnits = new ArrayList&lt;TimeUnit&gt;();

    public static void main(String[] args) {

        timeUnits.add(new Second());
        timeUnits.add(new Hour());
        timeUnits.add(new Minute());

        System.out.println(getTime("13:17:01"));
    }

    public static String getTime(String time) {

        StringBuilder timeSB = new StringBuilder();
        String[] timeElements = time.split(":");

        int i=0;
        int j=2;

        for(TimeUnit timeUnit : timeUnits){

            for(String timeElement : timeElements){
                timeSB.append(timeUnits.get(i).getLamps(Integer.parseInt(timeElements[j]))).append("\n");
                i++;
                j--;
                break;
            }
        }

        return timeSB.toString();
    }
}


TimeUnit

public interface TimeUnit {

    public String getLamps(int unit);

}


Hour

public class Hour implements TimeUnit {

    private static final String ALL_LIGHTS_OFF = "OOOO";
    private static final String RED_LIGHT = "R";
    private static final int MULTIPLE_OF_FIVE = 5;

    @Override
    public String getLamps(int hrs) {

        return getLampsForHoursMultiplesOfFive(hrs) + "\n" + getLampsForSingleHours(hrs);
    }

    public String getLampsForHoursMultiplesOfFive(int hours) {

        StringBuilder lamps = new StringBuilder(ALL_LIGHTS_OFF);

        for (int i = 0; i &lt; (hours / MULTIPLE_OF_FIVE); i++) {
            lamps.replace(i, i + 1, RED_LIGHT);
        }
        return lamps.toString();
    }

    public String getLampsForSingleHours(int hours) {

        StringBuilder lamps = new StringBuilder(ALL_LIGHTS_OFF);

        for (int i = 0; i &lt; (hours % MULTIPLE_OF_FIVE); i++) {  //dont do calculaion ever loop, do outside loop
            lamps.replace(i, i + 1, RED_LIGHT);
        }
        return lamps.toString();
    }
}


Minute

public class Minute implements TimeUnit {

    private static final String RED_LIGHT = "R";
    private static final String YELLOW_LIGHT = "Y";
    private static final String ALL_FOUR_LIGHTS_OFF = "OOOO";
    private static final String ALL_ELEVEN_LIGHTS_OFF = "OOOOOOOOOOO";

    @Override
    public String getLamps(int minutes) {

        int minutesDividedBy5 = minutes / 5;
        int minutesModulus5 = minutes % 5;

        return getLampsForMinutesMultiplesOfFive(minutesDividedBy5) + "\n" + getLampsForSingleMinutes(minutesModulus5);
    }

    private String getLampsForMinutesMultiplesOfFive(int minutes) {

        StringBuilder lamps = new StringBuilder(ALL_ELEVEN_LIGHTS_OFF);

        for (int i = 0; i &lt; minutes; i++) {
            if (0 == (i + 1) % 3) {
                lamps.replace(i, i + 1, RED_LIGHT);
            } else {
                lamps.replace(i, i + 1, YELLOW_LIGHT);
            }
        }
        return lamps.toString();
    }

    private String getLampsForSingleMinutes(int minutes) {

        StringBuilder lamps = new StringBuilder(ALL_FOUR_LIGHTS_OFF);

        for (int i = 0; i &lt; minutes; i++) {
            lamps.replace(i, i + 1, YELLOW_LIGHT);
        }
        return lamps.toString();
    }
}


Second

public class Second implements TimeUnit {

    @Override
    public String getLamps(int seconds) {

        if (0 == seconds%2) {
            return "Y";
        }

        return "O";
    }
}
That's a lot of code for something that could be done with a single function. The number of operations required to determine the output is relatively small, so writing a whole TimeUnit class hierarchy for it is overkill. Keeping your code short and simple makes it easier to understand, which helps preventing (or spotting) bugs.

If you do want to write a utility class, then a single, configurable TimeUnit class is sufficient. The only difference between the time units are lamp color and number of lamps per row, which can be passed in via the TimeUnit constructor.

Instead of passing strings around, why not use a built-in date/time type? That, or a set of 3 integers. That will clearly communicate how the function is to be used. A string is more ambiguous: should I pass in "15:30:05", "3:30:05PM", or some other format?

The 'S' in SOLID stands for Single responsibility. Let another function do the parsing. Your function should focus on the time-to-BerlinClock-display conversion. That's also related to 'I': Interface segregation. It's better to have several classes (or functions, in this case) that each do one thing well, than one big class/function that does everything, but turns out to be very inflexible when you only need part of that 'everything'.

Remember, KISS: Keep It Short and Simple. Also, DRY: Don't Repeat Yourself.