Question

Just a quick question look at the code below, is there any reason why wouldn't do this or is it fine?

public class MyClass implements Runnable, MyClassInterface {

    Thread threader;

    void start() {
       threader = new Thread(this);
       threader.start();
    }

    @Override
    public void run() {
       Thread current = Thread.getCurrentThread();
       while (threader = current) {
            ..
       }
    }
}

The original logic was not to expose that fact it runs in a separate thread to the caller who creates a "MyClass" but then there are doubts if that is a good thing or bad.

Can anyone see any good reason not to do it or is it acceptable. It can be expected that MyClass.start() maybe called a few times.

EDIT: Updated the code to show it is implementing Runnable and one other interface, the interface is used by client code, the actual implementation may run in a separate thread, same thread or any other way. The idea was to abstract that away from the client, as the client is simply an object that "MyClass" will notify and is not aware (currently) of the Runnable interface it implements.

Perhaps that abstraction is not needed and client should have more control?

EDIT: The start() was simply to tell the object it is ready to start receiving notifications rather than start a thread.

Have a look at this: http://docs.oracle.com/javase/8/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html

Was it helpful?

Solution

In my opinion, it is a bad design, because you are breaking encapsulation by implementing an interface (Runnable) and by providing a public method (run) that are of no use of the consumer of the class.

You can start a thread from the start method without inhering from Runnable:

public class MyClass {
    private Thread thread;
    public void start() {
       thread = new Thread(this::doWork); // Java 8 method reference
       thread.start();
    }
    private void doWork() {
        // ...
    }
}

If you can't use method references from Java 8, replace this::doWork with:

new Runnable() { public void run() { doWork(); } }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top