Is this timer any good?

Hi All,

I’ve been thinking about timers a bit lately and came up with this…


package org.kramer.utils;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

/**
 * Internally uses nanoseconds due to Java 1.5's System.nanoTime() being more
 * accurate. The public interface is in milliseconds though for convenience.
 * 
 * TODO: add support for one-off events, look into threading 
 * 
 * @author Kramer
 *
 */
public class Timer
{
	private long lastLoopTime = System.nanoTime();
	private List<Event> events = new ArrayList<Event>();

	/**
	 * Call this in the display() loop
	 */
	public void update()
	{
//		long deltaMillis = (System.nanoTime() - lastLoopTime) / 1000000;
		
		Iterator<Event> it = events.iterator();
		while (it.hasNext())
		{
			Event event = it.next();
			if (event.nextExecution <= System.nanoTime())
			{
				event.nextExecution = System.nanoTime() + (event.frequency * 1000000);
				event.doEvent();
			}
		}
		lastLoopTime = System.nanoTime();
	}
	
	/**
	 * @param frequency Frequency in milliseconds
	 * @param event
	 */
	public void scheduleEvent(long frequency, Event event)
	{
		event.frequency = frequency;
		event.nextExecution = System.nanoTime() + (frequency * 1000000);
		events.add(event);
	}
	

	/**
	 *
	 */
	public abstract class Event
	{
		long nextExecution;
		long frequency;
		
		public abstract void doEvent();
	}

}

…and in your init() you register events and how frequently you want them to happen as:


		Timer timer = new Timer();
		timer.scheduleEvent(500, timer.new Event(){
			public void doEvent() {
				System.out.println("every 500 milliseconds");
			}
		});
		timer.scheduleEvent(100, timer.new Event(){
			public void doEvent() {
				System.out.println("every 100 milliseconds");
			}
		});		

Then call timer.update() in display().

So… does anyone think this is any good? Would you use it? If not, please let me know why.

Thanks,
Kramer

Replace this:
event.nextExecution = System.nanoTime() + (event.frequency * 1000000);

with:
event.nextExecution += (event.frequency * 1000000);

to ensure it tries to execute at that interval, and can catch up if the cpu was too busy earlier.

Further ‘frequency’ is poorly named, it should be ‘interval’

Thanks Riven. I’ve taken your advice.