views:

63

answers:

3

Hi there :) I'm making a thread for my application that's going to do an exit operation at a given time (only hours and minutes, day/month doesn't matter). Is this the right way to do it, and also the right way to test for time? I'm testing for a 24 hour clock by the way, not AM / PM.

I'm then in another class going to call this something like new Thread(new ExitThread()).start();

public class ExitThread implements Runnable {



public long getDate() {
    Date thisdate = new Date(System.currentTimeMillis());
    return thisdate.getTime();
}


public long exitDate() {
    Date exitdate = new Date(System.currentTimeMillis());
    exitdate.setHours(23);
    exitdate.setMinutes(30);
    exitdate.setSeconds(0);

    return exitdate.getTime();
}

public long sleepTime() {
    Calendar cal = Calendar.getInstance();
    long now = cal.getTime().getTime();

    cal.set(Calendar.HOUR_OF_DAY, 23);
    cal.set(Calendar.MINUTE, 30);
    cal.set(Calendar.SECOND, 0);
    cal.set(Calendar.MILLISECOND, 0);

    long endMillis = cal.getTime().getTime();
    long timeToSleep = endMillis - now;

    return timeToSleep;
}

@Override
public void run() {

    while (true) {
        try {
            Thread.sleep(sleepTime());
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        if (getDate() >= exitDate()) {
            // System exit method here
        }
    }
}

}

+1  A: 

For testability, you should inject two things: a "sleeper" and a "clock". These could be within the same interface if you want, or separate. The production implementation would just use Thread.sleep and System.currentTimeMillis, but it means you can create fake implementations too, which make the code testable.

Jon Skeet
+1  A: 

You might want to recalculate currentTime within the while loop :-)

Instead of waiting 10 seconds before checking the end condition, you can determine the millis that correspond to your end time, and wait for the number of millies between now and the end time.

If your endtime would be between 235950 and 235959 you run the risk of missing it.

update

You can determine the number of millis to wait in this way:

Calendar cal = Calendar.getInstance();
long now = cal.getTime().getTime();

cal.set(Calendar.HOUR_OF_DAY,   23);
cal.set(Calendar.MINUTE,        30);
cal.set(Calendar.SECOND,        0);
cal.set(Calendar.MILLISECOND,   0);

long endMillis = cal.getTime().getTime();
long timeToSleep = endMillis - now;

Note that you need to calculate this within the while loop also, because the sleep can be interrupted the next iteration will need a smaller timeToSleep.

rsp
Updated the while loop :) Not sure how I would implement that last part though. Will give it a try.
DanielFH
Updated the code. Tested it and it works. Thanks for the help so far. Please let me know if you see any room for improvement :)
DanielFH
@DanielFH, as improvement you could get rid of the getDate and exitDate methods altogether by using `if (sleepTime() <= 0)`.
rsp
A: 

Perhaps you should consider using a Timer

Maurice Perry