4
\$\begingroup\$

I have written the following piece of code to have two threads printing odd and even numbers alternatively. I know this is a common question but I wanted to get this reviewed and understand if it is right or wrong. If wrong, can this be tweaked slightly to get desired result or the approach itself is wrong. Thanks for help.

public class ThreadTest implements Runnable{

    private static int counter = 1;
    private static final Object lock = new Object();

    public static void main(String[] args) {
        // TODO Auto-generated method stub

         Thread t1 = new Thread(new ThreadTest(), "arunabh");
         t1.start();
         Thread t2 = new Thread(new ThreadTest(), "hejib");
         t2.start();

    }    


@Override
public void run() {
    while (counter<=100) {
        synchronized (lock) {
            if (counter % 2 == 0) {
                System.out.println(counter +" Written By even Thread-"+ Thread.currentThread().getName());
                counter++;
                try {
                    lock.notify();
                    lock.wait();

                } catch (InterruptedException e) {
                    e.printStackTrace();
                }

            } else if (counter % 2 == 1) {
                System.out.println(counter +" Written By odd Thread-"+ Thread.currentThread().getName());
                counter++;

                try {
                     lock.notify();
                     lock.wait();

                } catch (InterruptedException e) {
                    e.printStackTrace();
                }

            }
        }
    }
  }
}
\$\endgroup\$
1
\$\begingroup\$

I have two comments of the tweaking kind:

1) any variable that is shared between threads has to be volatile. this is true even for static variables that have one value throughout the JVM (actually it is per class loader but this is irrelevant here). The reason for that is that threads can locally cache the value of variables. volatile variables force the thread to read the global value each time.

You might also want to take a look at AtomicInteger which is an int that is guaranteed to be incremented synchronously and that has the same value for all running threads.

2) while synchronized will work, Java concurrency package has several Lock mechanisms that allow for better control over the synchronization process. of relevancy here are ReentrantLock, ReadWriteLock.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ So if I make the static variable volatile and make the variable an AtomicInteger, is this program good enough to be written in an interview? Thanks for your answer. \$\endgroup\$ – Arunabh Jan 8 '18 at 9:27
  • \$\begingroup\$ I don't know about this specific scenario. \$\endgroup\$ – Sharon Ben Asher Jan 8 '18 at 9:49

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.