Improve accuracy of my timer thread

Question

Below I have a thread that updates a 'clock' label once per second. Once the thread reaches 600 (10 mins) the timer stops. The thread seems to trail by approx five seconds every minute. So when val = 60, in actuality 65 seconds may have elapsed. I'm using midp so I dont think I can introduce any api's to help me with this task. How can I improve on the accuracy below class? I think whats slowing it down is the method convertValToTimerString, is there a better way of converting the current timer val to minutes:seconds format without using java formatter apis?

Thanks,

public class LabelTimerUpdaterThread implements Runnable{

    public static int val = 0;
    private int minuteValueInt = 0;
    private int secondValueInt = 0;
    private int tempSecondValueInt;
    public boolean isRunning = true;

    public LabelTimerUpdaterThread(){

    }

    public void run() {
        while(isRunning){
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
            val += 1;


            incrementTimer();

            if(val == 600){
                break;
            }
        }
    }

    private void incrementTimer(){
        Screen.timerLabel.setText(convertValToTimerString(val));
    }

    private String convertValToTimerString(int val){

        String minuteValString = "";
        String secondValString = "";

        if(val < 10){
            minuteValString = "00";
            secondValString = "0"+String.valueOf(val);
            return minuteValString+":"+secondValString;
        }

        if(val < 60){
            minuteValString = "00";
            secondValString = String.valueOf(val);
            return minuteValString+":"+secondValString;
        }

        if(val % 60 == 0){
            ++minuteValueInt;
        }

            if(minuteValueInt < 10){
                minuteValString = "0"+String.valueOf(minuteValueInt);
                int secondVal = val % 60;
                if(secondVal < 10){
                    return minuteValString+":0"+String.valueOf(secondVal);
                }
                else {
                    return minuteValString+":"+String.valueOf(secondVal);
                }
            }

        return "10:00";
    }

    public void stopThread(){
        this.isRunning = false;
    }

Ok, now I am receiving an IllegalStateException when I try to upedate my timer label - Here is my code -

Here i am instantiating my label -

timerLabel = new CustomLabelField("00:00" , Field.FIELD_LEFT , Constants.SMALL_FONT , Color.BLACK, Bitmap.getBitmapResource("bg_clock_white.png"));

UpdateValTimer timer = new UpdateValTimer(th);
timer.startTimer();

This class creates the timer class and creates the class which will update the timer label.

public class UpdateValTimer {

    private Timer timer;
    private int val = 0;
    private UpdateView uv;
    private final CustomLabelField customLabelField;

    public UpdateValTimer(CustomLabelField field) {
        this.customLabelField = field;
    }

    public void startTimer(){
        timer = new Timer();
        uv = new UpdateView(customLabelField);
        Thread t = new Thread(uv);
        t.start();

        timer.scheduleAtFixedRate(new TimerTask() {
            public void run() {
                ++val;      
            }
        }, 1000, 1000);
    }
}

This class updates the timer label - public class UpdateView implements Runnable{

    private int val = 0;
    private int minuteValueInt = 0;
    private final CustomLabelField timerLabel;

    public UpdateView(CustomLabelField timerLabel) {
        this.timerLabel = timerLabel;
    }

    public void run() {
        while(true){
            this.timerLabel.setText(convertValToTimerString(this.val));
        }
    }

     private String convertValToTimerString(int val){

            String minuteValString = "";
            String secondValString = "";

            if(val < 10){
                minuteValString = "00";
                secondValString = "0"+String.valueOf(val);
                return minuteValString+":"+secondValString;
            }

            if(val < 60){
                minuteValString = "00";
                secondValString = String.valueOf(val);
                return minuteValString+":"+secondValString;
            }

            if(val % 60 == 0){
                ++minuteValueInt;
            }

                if(minuteValueInt < 10){
                    minuteValString = "0"+String.valueOf(minuteValueInt);
                    int secondVal = val % 60;
                    if(secondVal < 10){
                        return minuteValString+":0"+String.valueOf(secondVal);
                    }
                    else {
                        return minuteValString+":"+String.valueOf(secondVal);
                    }
                }

            return "10:00";
        }

}

Thanks for any help

After some initial testing this code seems to be working correctly.

Thanks for all your help everyone.

Any comments on what I could be doing better are welcome.

public class UpdateValTimer{

    private int minuteValueInt = 0;
    private Timer timer;
    private int val = 0;
    private UpdateView uv;
    private CustomLabelField customLabelField;

    public UpdateValTimer(CustomLabelField field) {
        this.customLabelField = field;
    }

    public void startTimer(){
        timer = new Timer();
            uv = new UpdateView(customLabelField);
            Thread t = new Thread(uv);
            t.start();

        timer.scheduleAtFixedRate(new TimerTask() {
            public void run() {             
                ++val;      
                uv.setVal(convertValToTimerString(val));
            }
        }, 1000, 1000);


    }

     private String convertValToTimerString(int val){

            String minuteValString = "";
            String secondValString = "";

            if(val < 10){
                minuteValString = "00";
                secondValString = "0"+String.valueOf(val);
                return minuteValString+":"+secondValString;
            }

            if(val < 60){
                minuteValString = "00";
                secondValString = String.valueOf(val);
                return minuteValString+":"+secondValString;
            }

            if(val % 60 == 0){
                ++minuteValueInt;
            }

                if(minuteValueInt < 10){
                    minuteValString = "0"+String.valueOf(minuteValueInt);
                    int secondVal = val % 60;
                    if(secondVal < 10){
                        return minuteValString+":0"+String.valueOf(secondVal);
                    }
                    else {
                        return minuteValString+":"+String.valueOf(secondVal);
                    }
                }

            return "10:00";
        }
}


public class UpdateView implements Runnable{

    private String timeElapsedCounter;
    private final CustomLabelField timerLabel;

    public UpdateView(CustomLabelField timerLabel) {
        this.timerLabel = timerLabel;
    }

    public void setVal(String timeElapsedCounter){
        this.timeElapsedCounter = timeElapsedCounter;
    }

    public void run() {

        while(true){
        synchronized(Application.getEventLock()){
            timerLabel.setText(this.timeElapsedCounter);
        }
        }
    }

}


        timerLabel = new CustomLabelField("00:00" , Field.FIELD_LEFT , Constants.SMALL_FONT , Color.BLACK, Bitmap.getBitmapResource("bg_clock_white.png"));

        UpdateValTimer timer = new UpdateValTimer(timerLabel);
        timer.startTimer();

Answer 1

There are two things that might help... First of all, the function incrementTimer() calls convertValToTimerString(). As you mentioned, this will introduce some slowdown. It will not be much, but over time this delay is going to accumulate. You're staying within the current thread of execution. A solution would be to use a model-view-controller scheme. The model would be your "val" field. The view is the label. The controller would then be the thread that updates this field once every second. By separating the model updates from the code execution required for viewing them, the update thread can run undisturbed. Of course, you'll still need a way to regularly update the text on the label based on your model's value. A separate thread could take care of this.

So what we have is:

• A model (in your case, simply one value)
• A controller (non-interactive thread updating the model)
• A view (label)
• A thread that updates the view

For the controller, using a Timer would indeed be a better choice rather than using a Runnable that calls Thread.sleep(), like andersoj suggested. You'll need to implement TimerTask. There is a difference between scheduling at fixed delay and scheduling at fixed rate. A fixed rate is preferrable for a task such as yours, where average consistency is more important than regularity. Do mind that if you use a Timer, the above model-view-controller scheme I suggested is a bit overkill. You'll probably be capable of incorporating the label update into the TimerTask's run method. But only because it will occur fast enough. If your view update requires more time (as might be the case for a combination of fast updates with heavy drawing) the MVC pattern will provide the proper separation of concerns and keep things spiffy.

Answer 2

Try using ScheduledExecutorService.scheduleAtFixedRate which will execute your task at fixed intervals, regardless of how long the previous task took to complete.

Answer 3

If you want a timer, suggest you use a Timer. Also suggest that if you want to maintain a clock, you not attempt to do this yourself, but instead sample the system clock. Perhaps what you want is to sample currentTimeMillis()

Answer 4

Thread.sleep(1000);

Will put the thread into the sleeping state for at least 1s and after that it gets back to ready-to-run state which doesn't guarantee it will run immediatelly, it is just "ready to be scheduled for running anytime in the future". It is up to the thread scheduler when it will run it. You may want for example to check the system time to really know how much time elapsed ...