1
\$\begingroup\$

I made this studying my Java book about concurrency. It's just to test. I feel it works ok but I'm not so sure about my Java. I worry maybe my code style is strange. I really didn't want that exception. It bothers me that I have to catch it.

I'd love to hear your input on the Java style used here. Any suggestion for how to do something better is welcome! Is it ok to make the array of threads like this?

package com.katie;

// Demonstration of a counter that is not thread-safe.
class Counter {
    private int myCounter = 0;
    public void increment() { myCounter++; }
    public int getCounter() { return myCounter; }
}

class BadIdea implements Runnable {
    private Counter myTarget;
    private int myStabs;
    BadIdea(Counter victim, int stabs) {
        myTarget = victim;
        myStabs = stabs;
    }
    public void run() {
        for (int i = 0; i < myStabs; i++) {
            myTarget.increment();
        }
    }
}

public class Main {
    public static void main(String[] args) {
        Counter guineaPig = new Counter();
        final int stabCount = 1000;
        final int threadCount = 3;

        // make multiple threads and use them to break the guineaPig
        // because she is not thread safe!

        Thread[] threads = new Thread[threadCount];
        for (int i = 0; i < threadCount; i++) {
            threads[i] = new Thread(new BadIdea(guineaPig, stabCount));
        }
        for (Thread t : threads) {
            t.start();
        }
        for (Thread t : threads) {
            try {
                t.join();
            } catch (InterruptedException e) {
                // i dont care!!
            }
        }

        // These will be different sometimes.

        System.out.println(guineaPig.getCounter());
        System.out.println(threadCount * stabCount);
    }
}
\$\endgroup\$
2
\$\begingroup\$

In general, your code looks fine to me. You could make myTarget and myStabs in BadIdea final. Also, you don't have to create three separate BadIdea objects in main(String[]). Instead, it would suffice to create only one BadIdea from guineaPig and stabCount and pass that object to the threads you start, because the BadIdea objects themselves will never change, only the Counter the point to, and since they all point to the same Counter (which is guineaPig), creating a separate BadIdea for each thread is redundant.

And about that InterruptedException: I can't envision a scenario in which an InterruptedException could ever be thrown in your program, but your comment "i dont care!!" implies that you think by merely swallowing the exception, you let the flow of your program be unaffected by it. However, this is not what would happen if an InterruptedException were actually thrown. If your code actually caught an InterruptedException, then it would no longer wait for t to die, but instead, the for-each loop would move on to the next thread, if there is one. So theoretically, it could happen that, if an InterruptedException is thrown, by the time you call System.out.println(guineaPig.getCounter()), not all 3 threads (or however many you start) are dead yet and the guinea pig is secretly still being tortured.

To prevent this from happening, you could rewrite the code to look something like this:

for (Thread t : threads) {
    while (t.isAlive()) {
        try {
            t.join();
        } catch (InterruptedException e) {
            // i really dont care!!
        }
    }
}
\$\endgroup\$
0
\$\begingroup\$
        try {
            t.join();
        } catch (InterruptedException e) {
            // i dont care!!
        }

It is common enough for java programmers to express this sentiment. The checked exceptions language "feature" isn't helping you in this case. Usually it makes sense to replace such a comment with throw new RuntimeException() to avoid silent surprises, such as the "would no longer wait for t to die" effect Stingy describes.

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