3

Is it a kind of bad design to use local Thread field in Runnable implemented class like this:

    public class TestingThread implements Runnable {
        Thread t;   

        public TestingThread() {
            t = new Thread(this, "Testing thread");

        }
        public void start()
        {
            t.start();
        }
...
Tony
  • 3,605
  • 14
  • 52
  • 84
  • Its certainly confusing to read, but this is mostly a matter of opinion and thus not a question for StackOverflow. – Gimby Jul 18 '14 at 12:32
  • A thread actually takes a `Runnable` as argument. So, *kind of* `yes` . I don't think keeping a thread reference as a field in a class implementing runnable is good. – TheLostMind Jul 18 '14 at 12:32
  • possible duplicate of [Why not to start a thread in the constructor? How to terminate?](http://stackoverflow.com/questions/5623285/why-not-to-start-a-thread-in-the-constructor-how-to-terminate) – earthmover Jul 18 '14 at 12:34
  • ...and [Java: starting a new thread in a constructor](http://stackoverflow.com/q/8057510/2764279) – earthmover Jul 18 '14 at 12:35
  • "Starting thread in the constructor" was not the topic of my question. I corrected my post to stop recommendations like these. – Tony Jul 18 '14 at 12:39
  • 1
    If you drew a graph of all of the object references in your program, it would have a cycle in it: The Thread object references the TestingThread object and vice versa. There's nothing _technically_ wrong with that, but you always should avoid creating cyclical dependencies. Back before anyone ever heard of "objects", we had a name for programs with descriptive graphs that had too many cycles: We called it "spaghetti code." Programs that were more tree-like were "structured code" which basically meant, "easier to understand." Those names are mostly forgotten, but the idea still is valid. – Solomon Slow Jul 18 '14 at 18:45

2 Answers2

2

Although this kind of question is obviously opinionated, I will tell you why I think that this is a design flaw. (Actually, I provoked this question, so maybe I should answer.)

A Runnable is an object holding the code that will (or should) be executed within a thread. The documentation explains it this way:

The Runnable interface should be implemented by any class whose instances are intended to be executed by a thread.

The main idea behind the Runnable interface is to separate the executable code from the means of executing it. There are several possibilities of running such code in an own thread.

One of those possibilities is the Thread class. From its documentation:

There are two ways to create a new thread of execution. One is to declare a class to be a subclass of Thread. This subclass should override the run method of class Thread. An instance of the subclass can then be allocated and started. [...] The other way to create a thread is to declare a class that implements the Runnable interface. That class then implements the run method. An instance of the class can then be allocated, passed as an argument when creating Thread, and started.

In my opinion, you always should do the latter. Define implementations of the Runnable interface, pass instances of them to the constructor of a Thread, then start the thread. Example:

class MyRunnable implements Runnable { ... }

void runMyCodeInOwnThread() {
    Runnable runnable = new MyRunnable();
    Thread thread = new Thread(runnable);
    thread.start();
}

The other way of executing a Runnable in a thread is an Executor, although you most likely will use an ExecutorService (actually a subtype of Executor). The utility class Executors will most likely be used for retrieving an executor service. Example:

class MyRunnable implements Runnable { ... }

void runMyCodeInOwnThread() {
    Runnable runnable = new MyRunnable();
    ExecutorService executor = Executors.newSingleThreadExecutor();
    Future<?> future = executor.submit(runnable); // or simply "executor.execut(runnable);"
}

If you don't tangle your runnable with the means of how to execute it, you are free to use either mechanism or change it in the future when requirements advance.

However, as I looked through the classes' documentations, I also saw the same mechanism in the examples. Still, I think this is not a good design.

Seelenvirtuose
  • 20,273
  • 6
  • 37
  • 66
  • +1, hard-wiring the technical means of execution to the task being executed is not a good thing. examples created with the intention of showcasing some feature for learning purposes may not be good resources for demonstrating real world usage patterns. – Nathan Hughes Jul 18 '14 at 14:15
1

Keeping or using a local thread variable inside another thread class is matter of choice.But personally from my knowledge its not a good design,until you have specific requirement to do so and use that local thread variable inside your thread class.You hardly needs this type of requirement in real life scenario.Hope it helps you !

Kuntal-G
  • 2,970
  • 16
  • 16