Convert a start and end date chosen by a user to epoch

Question 1

This method allows the user to choose a start and end date in human readable form, then convert it to epoch. The method will return a array of longs composed by the epoch start date and the epoch end date.

I handle some exception so if the user types anything else than the date it will end the iteration and start again. If the end date is before the start date it will start again…

private static long[] rectime_choice_period() {  
    long epochstart = 0;
    long epochend = 0;
    boolean confirm = true;

    String datetimeend = null;
    String datetimestart = null;

    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    while (confirm){

        //enter start date 
        System.out.print("nChoose start date (MM/dd/yyyy hh:mm:ss): ");     
        try{
            datetimestart = br.readLine();
        }catch (IOException e){
            System.err.println(e.getClass().getName()+": "+e.getMessage() + "nnn"); 
            continue;
        }

        //enter end date
        System.out.print("nChoose end date (MM/dd/yyyy hh:mm:ss): ");     
        try{
            datetimeend = br.readLine();
        }catch (IOException e){
            System.err.println(e.getClass().getName()+": "+e.getMessage() + "nnn"); 
            continue;
        }

        //convert to epoch
        try{
            epochstart = new java.text.SimpleDateFormat("MM/dd/yyyy HH:mm:ss").parse(datetimestart).getTime() / 1000;   
            epochend = new java.text.SimpleDateFormat("MM/dd/yyyy HH:mm:ss").parse(datetimeend).getTime() / 1000;    
        }catch (ParseException e){
            System.err.println("Error entered date cannot be converted!");
            continue;
        }catch (Exception e){
            System.err.println(e.getClass().getName()+": "+e.getMessage() + "nnn"); 
            continue;
        }


        if ((epochend - epochstart) > 0){
            System.out.println("Start time :" + epochstart + "tEnd time : " + epochend);
            confirm = false;
        }else {System.err.println("Wrong date! Start date should be before end date!");}

    }

    long[] period = {epochstart, epochend};
    return period;
}

But I am looking to optimize this code, maybe avoid the loop or the continue statement, I checked on google way to improve java code but nothing was really interesting. So any idea? Something I made wrong?

Question 2

The while loop isn’t really a problem. You need some way to loop back and re-ask the user input if something went wrong (e.g. they entered wrong data).

However, there are a couple of improvements that you can make:

You are opening a stream with

BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

but you are never closing it. This can lead to a resource leak. Starting with Java 7, you can deal easily with this by using the try-with-resources statement. Also, instead of using a BufferedReader, it would be easier to use a Scanner and its nextLine() method.

try (Scanner scanner = new Scanner(System.in)) {
    // read user input with scanner.readLine();
}

This has other advantages: you don’t need to catch any IOException so the code is cleaner.

try (Scanner scanner = new Scanner(System.in)) {
    while (confirm){
        System.out.print("nChoose start date (MM/dd/yyyy hh:mm:ss): ");     
        String datetimestart = scanner.nextLine();

        System.out.print("nChoose end date (MM/dd/yyyy hh:mm:ss): ");     
        String datetimeend = scanner.nextLine();
        // ...
    }
}

You should respect Java naming conventions for your variables and methods. Instead of datetimestart, prefer dateTimeStart (camel-case) and epochEnd. In the same way, instead of rectime_choice_period, prefer recTimeChoicePeriod (arguably, you could have another more descriptive name).

In the same way, you should respect the Java braces style

}else {System.err.println("Wrong date! Start date should be before end date!");}

should be rewritten

} else {
    System.err.println("Wrong date! Start date should be before end date!");
}

Don’t hardcode the date format "MM/dd/yyyy HH:mm:ss". You should refactor that into a constant instead:
```
private static final String DATE_FORMAT = "MM/dd/yyyy HH:mm:ss";
```
in this way, if the pattern should ever change, you don’t need to update the 4 occurences where it appears.
You are constructing twice a SimpleTextFormat when you could only construct one:
```
DateFormat formatter = new SimpleDateFormat("MM/dd/yyyy HH:mm:ss");
epochStart = formatter.parse(dateTimeStart).getTime() / 1000;   
epochEnd = formatter.parse(dateTimeEnd).getTime() / 1000;
```
As a side-note also, consider using imports instead of writing explicitely the fully qualified name of the class: import java.text.SimpleDateFormat;.
Avoid declarations of all the variables at the start of the method. Declare them only when they are needed. So, replace
```
String datetimeend = null;
// ...
datetimestart = scanner.nextLine();
```
with
```
String dateTimeEnd = scanner.nextLine();
```
You don’t need the parentheses inside if ((epochend - epochstart) > 0). A simple if (epochend - epochstart > 0) suffices and avoids extra parentheses.
Avoid useless comments: //enter start date adds no value as a comment. Good comments don’t repeat the code but clarify its intent.
Also, avoid useless temporary variables:
```
long[] period = {epochstart, epochend};
return period;
```
can just be written return new long[] { epochStart, epochEnd };.
In the code parsing the String into long, why are you catching the general Exception since you already have a catch clause for the ParseException? It is not necessary and should be removed.

Question 3

In the expression

(epochend - epochstart) > 0

the left hand expression can underflow, which makes it greater than 0 even though epochend < epochstart, which is what you actually want to test:

if (epochend > epochstart) {
    // success
} else {
    // failure
}

No over-/underflow issues and a much clearer way to express what you actually mean to test.

Leave a Reply Cancel reply