2
\$\begingroup\$

I have two threads. One runs a simulation and one runs a UI.

In the UI, it is possible to trigger a "tick" of the simulation. There are a few constraints:

  • Ticks run on the simulation thread
  • Each tick takes a bit of time to complete.
  • When the user asks for n ticks, n ticks should be completed, even if they are requested during the execution of a tick.

To manage the triggering of ticks between threads, I have created a Pulse class. Here it is:

public final class Pulse {

    private final Object Lock = new Object();

    private volatile int count;

    public Pulse() {

        super();

        count = 0;
    }

    public void pulse() {

        synchronized (Lock) {

            count++;

            Lock.notifyAll();
        }
    }

    public void waitForPulse() throws InterruptedException {

        synchronized (Lock) {

            while (count == 0) {

                Lock.wait();
            }

            count--;
        }
    }
}

A single Pulse instance is shared between the UI and simulation threads.

In the UI thread I would have something like:

void onTickRequested() {

    simulationPulse.pulse();
}

And in the simulation thread I have something like:

while (isRunningSimulation) {

    myPulse.waitForPulse();

    tick();
}

Will my code work as expected? Is it thread-safe?

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! What do you mean by, "Will my code work as expected?" Have you not tested your code? \$\endgroup\$ – SirPython Sep 23 '15 at 20:26
  • \$\begingroup\$ Well it works in my simple tests, but I don't know if I have applied the concurrency primitives correctly. \$\endgroup\$ – sdgfsdh Sep 23 '15 at 20:29
2
\$\begingroup\$

Good code! Just a few points:

  1. super();

    This line of code is automatically put in by the compiler and is unnecessary to write it yourself. If it is a habit that you have a hard time breaking, you may leave it, as it does not compromise speed, but I would leave it out, as it does clutter code slightly.

  2. Over-spacing

    Though spacing is good, too much space makes code hard to read as well. Your spacing is good, with one thing: you have a space between each line. Maybe it helps you, but I find it difficult to read.

    Compare:

    public final class Pulse {
    
        private final Object Lock = new Object();
    
        private volatile int count;
    
        public Pulse() {
    
            super();
    
            count = 0;
        }
    
        public void pulse() {
    
            synchronized (Lock) {
    
                count++;
    
                Lock.notifyAll();
            }
        }
    
        public void waitForPulse() throws InterruptedException {
    
            synchronized (Lock) {
    
                while (count == 0) {
    
                    Lock.wait();
                }
    
                count--;
            }
        }
    }
    

    and:

    public final class Pulse {
    
        private final Object Lock = new Object();
        private volatile int count;
    
        public Pulse() {
            super();
            count = 0;
        }
    
        public void pulse() {
            synchronized (Lock) {
                count++;
                Lock.notifyAll();
            }
        }
    
        public void waitForPulse() throws InterruptedException {
            synchronized (Lock) {
                while (count == 0) {
                    Lock.wait();
                }
                count--;
            }
        }
    
    }
    

    Note that I've kept some spaces.

  3. private final Object Lock = new Object();

    Java conventions state that variable names are camelCase, not PascalCase. Change Lock to lock.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.