1
\$\begingroup\$

I was asked below question in an interview where I need to print out even and odd numbers using two threads so I came up with below code one without synchronization and other with synchronization.

/**
 * Without synchronization
 *
 */
class Print {
  private static final int MAX = 10;
  private static int count = 1;
  private boolean isOdd = true;

  public void printEven() {
    while (true) {
      if (count > MAX)
        break;
      if (!isOdd) {
        System.err.println(Thread.currentThread().getName() + " : " + count++);
        isOdd = true;
      }
    }
  }

  public void printOdd() {
    while (true) {
      if (count > MAX)
        break;
      if (isOdd) {
        System.err.println(Thread.currentThread().getName() + " : " + count++);
        isOdd = false;
      }
    }
  }
}

/**
 * Using synchronization
 *
 */
class Print2 {
  private static final int MAX = 10;
  private static int count = 1;
  private Object obj = new Object();

  public void printEven() {
    while (true) {
      if (count > MAX)
        break;
      synchronized (obj) {
        System.err.println(Thread.currentThread().getName() + " : " + count++);
        obj.notify();
        try {
          obj.wait();
        } catch (InterruptedException e) {
        }
      }
    }
  }

  public void printOdd() {
    while (true) {
      if (count > MAX)
        break;
      synchronized (obj) {
        System.err.println(Thread.currentThread().getName() + " : " + count++);
        obj.notify();
        try {
          obj.wait();
        } catch (InterruptedException e) {
        }
      }
    }
  }
}


public class PrintEvenOddTester {
  public static void main(String[] args) {
    Print p = new Print();
    Thread t1 = new Thread(() -> p.printEven());
    t1.setName("EVEN");
    Thread t2 = new Thread(() -> p.printOdd());
    t2.setName("ODD");

    t1.start();
    t2.start();
  }
}

I wanted to check whether there is any better or efficient way to do these kind of tasks? I ran both the code and it works fine so wanted to check if everything looks good from thread safety and synchronization perspective.

Since in my above code, I am using while(true) loop which will take some CPU's so I am not sure whether this is the best way to do it?

\$\endgroup\$
5
  • \$\begingroup\$ Your first version operates on two global variables which are manipulated without any protection from thread switches. That means, the modification of count and isOdd is not guaranteed to be atomic at all. If you happen to have a thread switch between count++ and isOdd = ..., the whole system breaks. Your second version does not even work at all. Sorry, this is offtopic for codereview, as we only review working code here. \$\endgroup\$
    – mtj
    Oct 13 '18 at 5:58
  • \$\begingroup\$ @mtj why do you think second version doesn't work? I ran both the program in my eclipse and they all work as expected. There might be some thread safety issues as i am still learning that but they all are running and I get the right output as well. \$\endgroup\$ Oct 13 '18 at 6:26
  • \$\begingroup\$ I tried the second version here and the output was all over the place and totally incorrect. On retrying now, it seems OK. Sorry, might have been another problem \$\endgroup\$
    – mtj
    Oct 13 '18 at 7:19
  • \$\begingroup\$ @mtj can you upvote it please if you have downvoted? \$\endgroup\$ Oct 13 '18 at 16:26
  • 1
    \$\begingroup\$ "Since in my above code, I am using while(true) loop which will take some CPU" Any operation will 'take' some CPU or it wouldn't do us much good, would it? \$\endgroup\$
    – Mast
    Oct 15 '18 at 12:06
1
\$\begingroup\$

In Print both variables count and isOdd should be marked as volatile then it would be thread safe, but not very efficient.

In Print2 you assume that no one else could wake up your thread which is incorrect as thread could be awaked by many things - in such light you need to have additional if check there to check if it really should print the variable - like this isOdd in the Print. Having this wait/notify with a volatile isOdd check would be a solution that is both tread safe and efficient by not spinning the loops unnecessarily, count should also be volatile as it is shared beetwen threads.

If you want to learn java concurrency well you can read a book "Java Concurrency in Practice" - it will give you a good understanding of this subject and will allow you to pass inteview questions like this one.

You can ask here or on SO for specific things about java concurrency, but I think that it would be hard and inefficient for you to learn it this way. Much better (faster, less painfull) approach would be to read this book first and then ask questions if something will stay unclear for you. Good wishes!

\$\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.