views:

1285

answers:

9

is it legal for a thread to call this.start() inside its own constructor? and if so what potential issues can this cause? I understand that the object wont have fully initialized until the constructor has run to completion but aside from this are there any other issues?

+1  A: 

It's legal, but not wise. The Thread part of the instance will be completely initialised, but your constructor may not. There is very little reason to extend Thread, and to pull tricks like this isn't going to help your code.

Tom Hawtin - tackline
+1  A: 

Legal, yes. Good practice, no.

Michael Myers
A: 

agree about it being bad practice, but what exactly are the reprecussions? If this instantiated numerous times will it effect concurrency?

A: 

I assume that you want to do this to make your code less verbose; instead of saying

Thread t = new CustomThread();
t.start();
activeThreads.add(t);

you can just say

activeThreads.add( new CustomThread() );

I also like having less verbosity, but I agree with the other respondents that you shouldn't do this. Specifically, it breaks the convention; anyone familiar with Java who reads the second example will assume that the thread has not been started. Worse yet, if they write their own threading code which interacts in some way with yours, then some threads will need to call start and others won't.

This may not seem compelling when you're working by yourself, but eventually you'll have to work with other people, and it's good to develop good coding habits so that you'll have an easy time working with others and code written with the standard conventions.

However, if you don't care about the conventions and hate the extra verbosity, then go ahead; this won't cause any problems, even if you try to call start multiple times by mistake.

Eli Courtwright
A: 

I get ya. thanks for all the info guys!

This isn't supposed to be a forum. Its Q/A. Keep posts as answers.
Heath Borders
+2  A: 

By the way, if one wants lower verbosity and still keep the constructor with its "standard" semantics, one could create a factory method:

activeThreads.add( CustomThread.newStartedThread() );
millenomi
+1  A: 

It is "legal", but I think the most important issue is this: A class should do one thing and do it well.

If your class uses a thread internally, then the existence of that thread should not be visible in the public API. This allows improvement without affecting the public API. Solution: extend Runnable, not Thread.

If your class provides general functionality which, in this case, happens to run in a thread, then you don't want to limit yourself to always creating a thread. Same solution here: extend Runnable, not Thread.

For less verbosity I second the suggestion to use a factory method (e.g. Foo.createAndRunInThread()).

volley
+3  A: 

For memory-safety reasons, you shouldn't expose a reference to an object or that object's fields to another thread from within its constructor. Assuming that your custom thread has instance variables, by starting it form within the constructor, you are guaranteed to violate the Java Memory Model guidelines. See Brian Goetz's Safe Construction Techniques for more info.

Heath Borders
Comments on this answer? It is not incorrect.
Heath Borders
+1  A: 

You will also see wierd issues if the Thread class is ever further subclassed. In that case, you'll end up with the thread running already once the super() exits, and anything the subclass might do in its constructor could be invalid.

@bill barksdale If the thread is already running, calling start again gets you an IllegalThreadStateException, you don't get 2 threads.

John Gardner