Simple framework for looped applications

Question

I wrote up a quick API to help me save time while writing applications that require either a delay or frame rate. It should require me to implement a void run() method, which may or may not be annotated with the @Loop I created. If it doesn't contain the annotation, it will simply call the run() method once. If it is annotated, the client can specify either an fps (frames per second) or a delay (fps has priority over delay; if both are specified, fps will be used).

import vapp.App;
import vapp.Loop;

public class TestApp extends App {

    @Loop(fps=60)
    protected void run() {
        //my code
    }

    public static void main(String[] args) {
        launch();
    }
}

This is pretty straightforward for the client. I'll accept any suggestions on this part, but what I really want to get checked out is the framework itself.

• I'm not sure if my launch() method is breakable. I've tried breaking it, but couldn't find a way to. I haven't gotten any opinions on this yet, and I've been using it for a few months, so I would love to get some suggestions.
• I have AppEngine to encapsulate the Thread for the app, as well was the boolean that manages it. I then pass in my appLogic, which the logic handles:
  • Checking if the run() method is annotated with @Loop (if not, just call run()).
  • Check if a frame rate is specified. If so, start a loop that manages frame rate. If not, check if delay is specified; start a loop that simply sleeps for the delay. If a delay or fps isn't specified, start a loop with no limitations.
• Non-abstract method with no implementation. Is this the only way to ensure the subclass isn't forced to declare this method, but can if they want to?

I don't like how I create different conditional statements, each containing their own loop. I know something like this is logically needed, but it doesn't look elegant. I would love to clean it up.

Feel free to use these files for yourself if you find it functional.

Loop.java

package vapp;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface Loop {
    public int delay() default 0;
    public int fps() default 0;
}

App.java

package vapp;

import java.lang.reflect.Method;

public abstract class App {
    private AppEngine engine;

    public App() {
        engine = new AppEngine(appLogic);
    }

    protected static final void launch() {
        StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
        for(int i = 0; i < stackTrace.length; i++) {
            if(stackTrace[i].getClassName().equals(App.class.getName())) {
                try {
                    App app = (App) Class.forName(stackTrace[i+1].getClassName()).newInstance();
                    app.start();
                    return;
                } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
                    e.printStackTrace();
                }
            }
        }
    }

    private void start() {
        if (engine.isRunning())
            throw new IllegalStateException();

        init();
        engine.start();
    }

    private Runnable appLogic = () -> {
        try {
            Method runMethod = getClass().getDeclaredMethod("run");
            Class<Loop> loopClass = Loop.class;

            if(runMethod.isAnnotationPresent(loopClass)) {
                Loop loop = runMethod.getAnnotation(loopClass);
                int fps = loop.fps();
                int delay = loop.delay();

                try {
                    if(fps > 0) {
                        int timePerFrame = 1000/fps;
                        while(engine.isRunning()) {
                            long start = System.nanoTime();
                            run();
                            long sleepTime = (System.nanoTime() - start)/10000000 + timePerFrame;

                            if(sleepTime > 0) {
                                Thread.sleep(sleepTime);
                            } else {
                                Thread.yield();
                            }
                        }
                    } else if(delay > 0) {
                        while(engine.isRunning()) {
                            run();
                            Thread.sleep(delay);
                        }
                    } else {
                        while(engine.isRunning()) {
                            run();
                        }
                    }
                }catch(InterruptedException e) {
                    e.printStackTrace();
                }
            } else {
                run();
            }
        }catch(NoSuchMethodException e) {
            e.printStackTrace();
        }
    };

    protected void init() { };
    protected abstract void run();
}

AppEngine.java

package vapp;

import java.util.concurrent.atomic.AtomicBoolean;

public class AppEngine {
    private AtomicBoolean running;
    private Runnable appLogic;

    public AppEngine(Runnable appLogic) {
        this.appLogic = appLogic;

        running = new AtomicBoolean(false);
    }

    public final void start() {
        running.getAndSet(true);
        new Thread(appLogic).start();
    }

    public final void stop() {
        running.getAndSet(false);
    }

    public final boolean isRunning() {
        return running.get();
    }
}

I use an AtomicBoolean in case another thread attempts to stop the application at the same time the application attempts to stop itself. Is it overkill to use AtomicBoolean in this situation?

Answer 1

This idea is cool, I have some suggestions, though.

First off, you definitely should add JavaDoc to your annotation.

I wouldn't know that fps is preferred over delay, if you hadn't written it in your question (well I could find out, but it'd be a hassle). This should be done by documentation.
You can also document default values and usage constraints there, which brings me to the next point:

Your annotation only works if you annotate the run method. You might want to consider changing that a little and grab the method depending on the annotation you have (and not check for the annotation on your run method).
But that's mostly up to you ;)

Next thing I want to talk about is the units you're using.

Your delay is an int. I strongly assume the delay is given in milliseconds. If you'd want to reuse this it might be interesting to allow specifying the delay with an arbitrary TimeUnit, but that's future talk.

What's more interesting is things like this code:

int timePerFrame = 1000 / fps;

time as what? be explicit and tell the reader exactly what you mean and say: msPerFrame

It's somewhat similar when you grab the sleepTime. When first reading this I was somewhat.. confused. Tell the reader you need to divide by 1 mil. because you need to get from nanoseconds to milliseconds.

BUG ALERT

Btw, that's a bug right there. Counting the zeroes I'm getting to 10 mil. instead of 1. To prevent this you can write numeric literals with underscores in between them for easier overview:

long sleepTime = (System.nanoTime() - start) / 1_000_000 + msPerFrame;

BUG EXTERMINATION COMPLETE

Moving swiftly on to how you handle delay. It's very interesting to see the difference in how you solved the runtime of run problem from fps to delay.

I'd have expected you to do the same thing as with the fps. Instead you wait for run to complete before counting down the delay. If you want to have the same handling so fps and delay are usable interchangeably, you should do the following:

while (engine.isRunning()) {
     long start = System.nanoTime();
     run();
     long remainingDelay = delay - ((System.nanoTime() - start) / 1_000_000);
     
     if (remainingDelay > 0) {
        Thread.sleep(remainingDelay);
     } else {
        Thread.yield();
     }
}

which actually was another (rather minor) bug.

Movig swiftly on to your AppEngine, where I see you do the following twice:

running.getAndSet(...);

Which makes me ask myself: Why not just use set() or lazySet()? It seems you used to return booleans, eh? Oh and another thing:

If you already make all the methods you have final, why not also make the class final? Then you can even remove the final from all the methods you have ;)