Seconds into readable time information

Question

I had an exercise to solve, written in German. I try to translate it:

Write a program that takes an amount of seconds. The program than has to output the number of years, days, hours and seconds.

My Solution

public class Main {
  public static void main(String[] args) {
    final int HOUR_IN_SECONDS = 60 * 60;
    final int DAY_IN_SECONDS = HOUR_IN_SECONDS * 24;
    final int YEAR_IN_SECONDS = DAY_IN_SECONDS * 365;

    int seconds = 1000000000;

    // Jahre berechnen
    int years = seconds / YEAR_IN_SECONDS;
    seconds = seconds - years * YEAR_IN_SECONDS;
    System.out.println("Jahre: " + years);

    // Tage berechnen
    int days = seconds / DAY_IN_SECONDS;
    seconds = seconds - days * DAY_IN_SECONDS;
    System.out.println("Tage: " + days);

    // Stunden berechnen
    int hours = seconds / HOUR_IN_SECONDS;
    seconds = seconds - hours * HOUR_IN_SECONDS;
    System.out.println("Stunden: " + hours);

    // Minuten berechnen
    int minutes = seconds / 60;
    seconds = seconds - minutes * 60;
    System.out.println("Minuten: " + minutes);

    // Sekunden ausgeben
    System.out.println("Sekunden: " + seconds);
  }
}

This code works, but is it also beautiful enough and easy to read?

Answer 1

I have some suggestion for you.

1) The naming convention of the variables HOUR_IN_SECONDS, DAY_IN_SECONDS and YEAR_IN_SECONDS is not correct, since they are not constant; missing the static keyword. I suggest that you extract them in the class level.

public static final int HOUR_IN_SECONDS = 60 * 60;
public static final int DAY_IN_SECONDS = HOUR_IN_SECONDS * 24;
public static final int YEAR_IN_SECONDS = DAY_IN_SECONDS * 365;

public static void main(String[] args) {
   //[...]
}

2) I suggest that you create a method that take the number of seconds and print the result.

public static void main(String[] args) {
  int seconds = 1000000000;
  printFromSeconds(seconds);
}

private static void printFromSeconds(int seconds) {
    //[...]
}

For the rest, I think it's ok since the variable seconds is closely linked with the other components.

Answer 2

First off a typical bad habit many beginners get into is, putting everything in main. If the code is only doing one thing and can be self contained a function will do. Otherwise, use a class. In this case I would suggest a class with static fields and functions.

Try to avoid using magic numbers, strings, etc.. Use final variables so that they can be named. In many cases it can be difficult to figure out why a particular value was used, in a years time or longer.

There is a fair bit of repetition in your code. A helper function would clean that up.

Use formatted strings instead concatenation(+).

Since you seem to be using one language for programming and a different one for output, I would deduce that it's possible that other languages may also be required for the output. to this end a simple TimeNames class to store different string arrays with the time labels in different languages.

TimeNames.java

public class TimeNames{
    public enum Languages{
        English,
        German
    }
    static final String[] germanNames = {
        "Jahre",
        "Tage",
        "Stunden",
        "Minuten",
        "Sekunden"
    };
    static final String[] englishNames = {
        "Years",
        "Days",
        "Hours",
        "Minutes",
        "Seconds"
    };
    static String[] names = englishNames;    

    public static void changeLanguage(Languages language){
        switch(language){
            case English:
            names = englishNames;
            break;
            case German:
            names = germanNames;
            break;
            default:
            names = englishNames;
        }       
    }
    public static String getYears(){
        return names[0];
    }
    public static String getDays(){
        return names[1];
    }
    public static String getHours(){
        return names[2];
    }
    public static String getMinutes(){
        return names[3];
    }
    public static String getSeconds(){
        return names[4];
    } 
}

SecondsToTime.java

public class SecondsToTime{
    static final int SECS_IN_MINUTES = 60;
    static final int MINS_IN_HOUR = 60;
    static final int HOURS_IN_DAY = 24;
    static final int DAYS_IN_YEAR = 365;

    static final int MIN_CONV = SECS_IN_MINUTES;
    static final int HOUR_CONV = MIN_CONV * MINS_IN_HOUR;
    static final int DAY_CONV = HOUR_CONV * HOURS_IN_DAY;
    static final int YEAR_CONV = DAY_CONV * DAYS_IN_YEAR;

    static long years = 0;
    static long days = 0;
    static long hours = 0;
    static long minutes = 0;
    static long seconds = 0;

    public static String secondsToTime(long secs){
        long[] secsTotal = {secs};
        years = convert(YEAR_CONV,secsTotal);
        days = convert(DAY_CONV,secsTotal);
        hours = convert(HOUR_CONV,secsTotal);
        minutes = convert(MIN_CONV, secsTotal);
        seconds = secsTotal[0];
        return String.format("%s: %d\n%s: %d\n%s: %d\n%s: %d\n%s: %d", TimeNames.getYears(), years, TimeNames.getDays(), days,
                TimeNames.getHours(), hours, TimeNames.getMinutes(), minutes, TimeNames.getSeconds(), seconds);
    }

    private static long convert (final int factor, long[] secs){
        int temp = (int)secs[0]/factor;
        secs[0] -= temp * factor;
        return temp;
    }
}

Main.java

public class Main{
    public static void main(String[] args) {
        String time = SecondsToTime.secondsToTime(1000000000);
        System.out.println(time);
        TimeNames.changeLanguage(TimeNames.Languages.German);
        time = SecondsToTime.secondsToTime(1000000000);
        System.out.println(time);
    } 
}

Answer 3

I found a little inconsistency in the following part of the code:

// Minuten berechnen
int minutes = seconds / 60;
seconds = seconds - minutes * 60;
System.out.println("Minuten: " + minutes);

In all the other paragraphs you use named constants. You should follow that pattern here as well. MINUTE_IN_SECONDS is a perfect name for the 60.

The following two statements are equivalent. The second is idiomatic Java, the first is unnecessarily verbose.

seconds = seconds - minutes * 60;
seconds -= minutes * 60;

In the following declaration, I prefer the second variant since it is closer to the natural language "1 day is 24 hours" when being read aloud.

int DAY_IN_SECONDS = HOUR_IN_SECONDS * 24;
int DAY_IN_SECONDS = 24 * HOUR_IN_SECONDS;

A completely different way of splitting the total seconds x into the different time units is:

int seconds = x % 60;
x /= 60;

int minutes = x % 60;
x /= 60;

int hours = x % 24;
x /= 24;

int days = x % 365;
x /= 365;

int years = x;

This way you don't need the named constants since there are 60 seconds, 24 hours and 365 days, and all the numbers that are spelled out are written close to the name of their corresponding time unit.

Sure, the code looks like it has a lot of duplication, but there's no easy way around that since Java doesn't support multiple return values per method.