2
\$\begingroup\$

I have written a little program that calculates the time by given strings.

A time string in the context of this program consists of a number and a following letter that can be s, m, h, or d.

You can enter something like this:

java TimeCalculator 443432s 34234m 34323m 32344234s 8h 23d 12h

It than calculates the time and prints it out in an readable format:

450 days 22 hours 18 minutes 6 seconds 

I want to ask you, how I can improve the readability and maybe also the performance. I dont tried to design the program object oriented because I think the program is to small for that. But if you have another opinion on that, please let me know!

public class TimeCalculator {
    private static final int MINUTE_IN_SECONDS = 60;
    private static final int HOUR_IN_SECONDS = 3600;
    private static final int DAY_IN_SECONDS = 86400;

    private static int seconds = 0;

    public static void main(String[] args) {
        if (args.length == 0 || args[0].contains("help")) {
            showHelp();
            return;
        }

        for (String arg : args) {
            timeStringToSeconds(arg);
        }

        printNewTimeString();
    }

    private static void timeStringToSeconds(String timeString) {
        if (timeString.contains("s")) {
            timeString = timeString.replace("s", "");
            seconds += Integer.parseInt(timeString);
        } else if (timeString.contains("m")) {
            timeString = timeString.replace("m", "");
            seconds += Integer.parseInt(timeString) * MINUTE_IN_SECONDS;
        } else if (timeString.contains("h")) {
            timeString = timeString.replace("h", "");
            seconds += Integer.parseInt(timeString) * HOUR_IN_SECONDS;
        } else if (timeString.contains("d")) {
            timeString = timeString.replace("d", "");
            seconds += Integer.parseInt(timeString) * DAY_IN_SECONDS;
        }
    }

    private static void printNewTimeString() {
        String newTimeString = new String();
        // calculate days
        int days = 0;
        while (seconds >= DAY_IN_SECONDS) {
            days++;
            seconds -= DAY_IN_SECONDS;
        }
        if (days > 0) {
            newTimeString += days + " days ";
        }

        // calculate hours
        int hours = 0;
        while(seconds >= HOUR_IN_SECONDS) {
            hours++;
            seconds -= HOUR_IN_SECONDS;
        }
        if (hours > 0) {
            newTimeString += hours + " hours ";
        }

        // calculate minutes
        int minutes = 0;
        while (seconds >= MINUTE_IN_SECONDS) {
            minutes++;
            seconds -= MINUTE_IN_SECONDS;
        }
        if (minutes > 0) {
            newTimeString += minutes + " minutes ";
        }

        // calculate seconds
        if (seconds > 0) {
            newTimeString += seconds + " seconds ";
        }

        System.out.println(newTimeString);
    }

    private static void showHelp() {
        System.out.println("This Program converts time strings to an ordered string that makes the time");
        System.out.println("information better understandable\n");
        System.out.println("You can enter something like this: ");
        System.out.println("java TimeCalculator 534s 400d 32453s 234h");
        System.out.println("The output will be this: ");
        System.out.println("410 days 3 hours 9 minutes 47 seconds");
        System.out.println("Available characters: d, h, m, s");
    }
}
\$\endgroup\$
2
\$\begingroup\$

A few things I'd like to point out:

Whenever you're code gets input from a user of your code you must have some way of verifying that they're entering the right type of data and in the right format. not having this is just looking for trouble.

Using a switch block is usually much easier to work with than a bunch of if blocks.

There is a Duration class in the time api which is made for this type of situation. It is made to help with time calculations as well date/time parts.

Here's one way all this could be put together:

import java.time.Duration;

public class TimeCalculator {


    public static void main(String[] args) {
        if (args.length == 0 || args[0].contains("help")) {
            showHelp();
            return;
        }
        try{
          System.out.println(GetFormattedString(args));          
        }catch(Exception e){
            System.out.println(e.getMessage());
        }
    }

    private static String GetFormattedString(String[] times) throws NumberFormatException, IllegalArgumentException  {
        Duration totalTime = makeTime(times);
        String test = totalTime.toString();
        long days = totalTime.toDaysPart();
        int hours = totalTime.toHoursPart();
        int minutes = totalTime.toMinutesPart();
        int seconds = totalTime.toSecondsPart();
        return String.format("%1$d days %2$d hours %3$d minutes %4$d seconds", days,hours,minutes,seconds);
    }

    private static Duration makeTime(String[] times) throws NumberFormatException, IllegalArgumentException  {
        Duration totalTime = Duration.ZERO;
        for (  var s:times) {
            long time = 0;
            int lastIndex = s.length()-1;
            try {
                time = Integer.valueOf(s.substring(0, lastIndex));
            } catch (NumberFormatException e) {
                throw new NumberFormatException("Invalid characters present in strings provided");
            }
            switch (s.charAt(lastIndex)) {
                case 's':
                    totalTime = totalTime.plusSeconds(time);
                    break;
                case 'm':
                    totalTime = totalTime.plusMinutes(time);
                    break;
                case 'h':
                    totalTime = totalTime.plusHours(time);
                    break;
                case 'd':
                    totalTime = totalTime.plusDays(time);
                    break;
                default:
                    throw new IllegalArgumentException("String provided is in the wrong format");
            }
        }
        return totalTime;
    }

    private static void showHelp() {
        System.out.println("This Program converts time strings to an ordered string that makes the time");
        System.out.println("information better understandable\n");
        System.out.println("You can enter something like this: ");
        System.out.println("java TimeCalculator 534s 400d 32453s 234h");
        System.out.println("The output will be this: ");
        System.out.println("410 days 3 hours 9 minutes 47 seconds");
        System.out.println("Available characters: d, h, m, s");
    }
}

EDIT

I didn't really like throwing exceptions like that, so I came up with a version that eliminates throwing exceptions. I added 2 things, a tryParseLong method and a wrapper class(TimeLong) to be able to pass the time value by reference:

import java.time.Duration;

public class TimeCalculator {

    public static void main(String[] args) {
        if (args.length == 0 || args[0].contains("help")) {
            showHelp();
            return;
        }
        String formattedString = GetFormattedString(args);
        if(formattedString.equals("")){
            System.out.println("Invalid arguments used");
            showHelp();
            return;
        }
        System.out.println(formattedString);
    }

    private static String GetFormattedString(String[] times) {
        Duration totalTime = makeTime(times);
        if (totalTime == Duration.ZERO) {
            return "";
        }
        long days = totalTime.toDaysPart();
        int hours = totalTime.toHoursPart();
        int minutes = totalTime.toMinutesPart();
        int seconds = totalTime.toSecondsPart();
        return String.format("%1$d days %2$d hours %3$d minutes %4$d seconds", days, hours, minutes, seconds);
    }

    private static Duration makeTime(String[] times) {
        Duration totalTime = Duration.ZERO;
        for (var s : times) {
            TimeLong time = new TimeLong((0L));
            int lastIndex = s.length() - 1;
            if (!tryParseLong(s.substring(0, lastIndex), time)) {
                return Duration.ZERO;
            }
            switch (s.charAt(lastIndex)) {
                case 's':
                    totalTime = totalTime.plusSeconds(time.value);
                    break;
                case 'm':
                    totalTime = totalTime.plusMinutes(time.value);
                    break;
                case 'h':
                    totalTime = totalTime.plusHours(time.value);
                    break;
                case 'd':
                    totalTime = totalTime.plusDays(time.value);
                    break;
                default:
                    return Duration.ZERO;
            }
        }
        return totalTime;
    }
    private static class TimeLong{
        public Long value;
        public TimeLong(Long value){
            this.value= value;
        }
    }
    private static boolean tryParseLong(String value, TimeLong outVal) {
        try {
            outVal.value =  Long.valueOf(value);
        } catch (Exception e) {
            outVal.value = 0L;
            return false;
        }
        return true;
    }

    private static void showHelp() {
        System.out.println("This Program converts time strings to an ordered string that makes the time");
        System.out.println("information better understandable\n");
        System.out.println("You can enter something like this: ");
        System.out.println("java TimeCalculator 534s 400d 32453s 234h");
        System.out.println("The output will be this: ");
        System.out.println("410 days 3 hours 9 minutes 47 seconds");
        System.out.println("Available characters: d, h, m, s");
    }
}
\$\endgroup\$
  • \$\begingroup\$ I get several error messages when I try to compile your code (using Java 8) \$\endgroup\$ – Dexter Thorn Dec 12 '18 at 8:47
  • \$\begingroup\$ @DexterThorn - I used the latest NetBeans with Java 11 to build and run this. toDaysPart() was added in Java 9. To use java 8, you need to use toDays() and then minus that amount from totalTime, and repeat for each time part. If you still have a problem let me know and I can add some code for that. \$\endgroup\$ – tinstaafl Dec 12 '18 at 15:05
3
\$\begingroup\$

Instead of subtracting the day_in_seconds repeatedly you could divide the total seconds by day_in_seconds and then set seconds equal to (seconds % days_in_seconds).

Something like:

if(seconds/days_in_seconds > 0) {
    days = seconds/days_in_seconds;
    seconds = seconds%days_in_seconds;
}

And so on for the other values..

Hope that helps (Posting from mobile)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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