this.wait( sleep );

[Ricardo]

And I've very convinced that exactly that kind of code opens things up
to more maintenance errors.

Exactly how do you find Thread.currentThread().sleep(1000) a potential
bug or error? Have you worked with production source code authored by
Sun, Apache/Jakarta, or even IBM? If you argument is another repeat of
"This is not how you call a static method", then that's been answered in
more than one occasion. The discussion is related to this and only this
class: Thread.

Go lookup IBM's article on the 'rouge tile' bug pattern for an example
of the kind of thing I'm talking about.

If you feel very strongly about that article, perhpas you can at least
provide a link.

 

[Jon Skeet]

I know that Eclipse complains about it. You can set that as an option in
the preferences, so I'd imagine you could turn that warning on with an
option to the compiler.

Not to the normal Java compiler, no. Eclipse uses its own compiler,
which is how it is able to add warnings like this.

 

[Jon Skeet]

Absolutely not..

The only exceptions: Thread.sleep(..), Thread.yield(),
Thread.holdsLock(..) and Thread.dumpStack().

If those methods were named more properly (i.e., yieldCurrentThread()),
or made part of a CurrentThread helper class, you'd be proud of how I
make those calls.

I really don't think it's necessary though. Being static, if anyone
*is* confused, they'll *know* they're confused immediately, and can
consult the documentation to become unconfused. With your way of
writing code, people are likely to get confused and *stay* confused
about what Thread.sleep does.

I'm convinced that Thread.currentThread().sleep(..) would give the exact
impression as to which thread is being affected. I know this is not
what you're saying, but it's exactly what's intended by this funny code.

It would give the right impression about what is happening, but the
wrong impression about what the Thread.sleep method itself does. Ask
yourself which convention is more likely to lead to the reader
eventually writing code such as:

Thread t = new Thread (....);
t.start();
t.sleep(500);

I'm sure that neither you nor Chris would design a class with an
interface similar to that of Thread. If you were to commit such an act,
and if I were the user of that class, I'd be committing that same sin again!

Actually, it's a common design for thread classes, I believe - static
methods apply to the current thread, because that's the only thread it
makes sense for them to apply to. Instance methods apply to the method
they're called on. That's applied consistently - contrary to your post
a while ago which claimed that isInterrupted gives information about
the current thread, it gives information about the thread instance it's
called on.

I don't find it in the least bit confusing - but I'm sure I would if I
were a new programmer who encountered code which called the static
method via an instance reference.

 

[Jon Skeet]

Exactly how do you find Thread.currentThread().sleep(1000) a potential
bug or error?

It encourages people who don't know any better to think that they can
call sleep on a different thread.

Have you worked with production source code authored by
Sun, Apache/Jakarta, or even IBM?

How exactly is that relevant?

If you argument is another repeat of
"This is not how you call a static method", then that's been answered in
more than one occasion. The discussion is related to this and only this
class: Thread.

Applying a bad pattern in just one place doesn't make it a better
pattern.

If you feel very strongly about that article, perhpas you can at least
provide a link.

Two google searches later:

http://www-106.ibm.com/developerworks/library/j-diag1.html

 

[Collin VanDyck]

Not to the normal Java compiler, no. Eclipse uses its own compiler,

which is how it is able to add warnings like this.

Ah, thanks for the info. I had always assumed Eclipse used whatever javac
was on the path.

-CV

 

[P.Hill]

Ah, thanks for the info. I had always assumed Eclipse used whatever javac
was on the path.

This is also how you can introduce errors into code, save the file
and debug right into the class without stopping unless you
actually try to execute the bad code at which point Eclipse stops.

Apparently Eclipse inserts an exception throwing place holder.

-Paul

 

[Ricardo]

I really don't think it's necessary though. Being static, if anyone
*is* confused, they'll *know* they're confused immediately, and can
consult the documentation to become unconfused. With your way of
writing code, people are likely to get confused and *stay* confused
about what Thread.sleep does.

I'll borrow your example from below..

Thread t = new Thread (....);
t.start();
t.sleep(500);

As long as this compiles, then the problem exists. There is nothing that
would stop anyone from writing it. The confusion is not the result from
seeing Thread.currentThread().sleep(..), but from having start,
interrupt, yield, sleep, ... as methods in the same class that can do
very unpredictable things..

Now, let's put the novice hats on and compare *readability*:

// A
Thread t = new Thread (....);
t.start();
Thread.currentThread().sleep(500);

// B
Thread t = new Thread (....);
t.start();
Thread.sleep(500);

Now, consider renaming t to thread..

// C (common? oh yes)
Thread thread = new Thread (....);
thread.start();
Thread.sleep(500);

Does A encourages t.sleep()? I don't see that. However, A is by far
the most readable.

It would give the right impression about what is happening, but the
wrong impression about what the Thread.sleep method itself does. Ask
yourself which convention is more likely to lead to the reader
eventually writing code such as:

Thread t = new Thread (....);
t.start();
t.sleep(500);

And that's the heart of the issue..
If it's a way to learn the API, you score all the points.
However, I believe strongly that this class in particular is critical
enough that I'd make sure that sleep in only called using a reference to
currentThread.

Actually, it's a common design for thread classes, I believe - static
methods apply to the current thread, because that's the only thread it
makes sense for them to apply to. Instance methods apply to the method
they're called on. That's applied consistently - contrary to your post
a while ago which claimed that isInterrupted gives information about
the current thread, it gives information about the thread instance it's
called on.

You're referencing the following:

and that was a bad comment, resulted from cut+paste from previous line.
It should've been: "/* instance method */". Debugging comments is far
more difficult than debugging code.

However, you did touch another call that you find debated in lunch rooms:

Thread.interrupted()

vs.

Thread.currentThread().isInterrupted()

I do prefer the latter (to the surprise of few), and hope this won't be
questioned by the static methods police..

I don't find it in the least bit confusing - but I'm sure I would if I
were a new programmer who encountered code which called the static
method via an instance reference.

Jon, maybe you don't find it confusing, but it is a nasty design. At
any given point, you could call start, stop, pause, interrupt, sleep,
yield, all as methods in the same class, all are legal when called using
an instance. That would certainly not be your design.

 

[Ricardo]

It encourages people who don't know any better to think that they can
call sleep on a different thread.

This is certainly valid, and a good example is indeed shown by Gordon,
few posts to the east..

However, in no way it is a bug or an error.

How exactly is that relevant?

Just because you'd find this call made many many times (did I stress
many?). I picked those three because I'm sure people would find sources
on their systems, and they'll come to realize it's all over the place.
Some here behave as if this is the first they've seen this, and that's
what gets me boiling..

Applying a bad pattern in just one place doesn't make it a better
pattern.

The point is not to make this a better pattern. It's simply a self
documenting piece of code that insures and guarantees the reader what
the effects of this call are.

Two google searches later:

http://www-106.ibm.com/developerworks/library/j-diag1.html

Thank you for the effort; it should've been Cruz though who should do
the work

 

[Jon Skeet]

I'll borrow your example from below..

Thread t = new Thread (....);
t.start();
t.sleep(500);

As long as this compiles, then the problem exists. There is nothing that
would stop anyone from writing it. The confusion is not the result from
seeing Thread.currentThread().sleep(..), but from having start,
interrupt, yield, sleep, ... as methods in the same class that can do
very unpredictable things..

They don't do unpredictable things at all. The confusion is the result
of being able to call static methods as if they were instance methods,
which then leads people to think that they operate on the instance
they're talking about.

Now, let's put the novice hats on and compare *readability*:

// A
Thread t = new Thread (....);
t.start();
Thread.currentThread().sleep(500);

// B
Thread t = new Thread (....);
t.start();
Thread.sleep(500);

Now, consider renaming t to thread..

// C (common? oh yes)
Thread thread = new Thread (....);
thread.start();
Thread.sleep(500);

Does A encourages t.sleep()? I don't see that.

Novice programmer reads your code. He hasn't encountered Thread.sleep
before. He reads the code and thinks, "Ah, Thread.sleep must be an
instance method which sends whatever thread it's called on to sleep."

Later, when he's writing some other code where he (foolishly) wants to
make another thread pause, he calls t.sleep() and then gets confused
when it makes "the wrong thread" sleep.

However, A is by far the most readable.

In your view - not in mine. It gives a false impression of the nature
of the Thread.sleep call, in my view - and that doesn't add to
readability, it reduces it.

And that's the heart of the issue..
If it's a way to learn the API, you score all the points.

If the programmer knows the API already, then it's irrelevant, as
they'll know that Thread.sleep makes the current thread sleep.

However, I believe strongly that this class in particular is critical
enough that I'd make sure that sleep in only called using a reference to
currentThread.

*You* might do that - but will everyone else who's read your code and
got the wrong end of the stick?

You're referencing the following:


and that was a bad comment, resulted from cut+paste from previous line.
It should've been: "/* instance method */". Debugging comments is far
more difficult than debugging code.

Right. However, it does mean that the Thread class is entirely
consistent.

However, you did touch another call that you find debated in lunch rooms:

Thread.interrupted()

vs.

Thread.currentThread().isInterrupted()

I do prefer the latter (to the surprise of few), and hope this won't be
questioned by the static methods police..

I don't particularly mind either of them. What I *would* object to is

Thread.currentThread().interrupted()

as again, that gives a false impression.

Jon, maybe you don't find it confusing, but it is a nasty design. At
any given point, you could call start, stop, pause, interrupt, sleep,
yield, all as methods in the same class, all are legal when called using
an instance. That would certainly not be your design.

My design would be for the language to forbid calling
Thread.currentThread().sleep() in the first place (as C# does, btw).
From that starting point, I see nothing particularly wrong in the
design of the Thread class. The "all static methods operate on the
current thread; all instance methods operate on the instance thread"
rule is hardly difficult to learn.

 

[Jon Skeet]

This is certainly valid, and a good example is indeed shown by Gordon,
few posts to the east..

However, in no way it is a bug or an error.

It *encourages* bugs though. Giving people the idea that a method
operates on whatever object you tell it to when in fact it doesn't is a
fundamentally bad idea, because sooner or later they *will* call it
expecting it to operate on the object they specify which doesn't happen
to be "the right one".

Just because you'd find this call made many many times (did I stress
many?). I picked those three because I'm sure people would find sources
on their systems, and they'll come to realize it's all over the place.
Some here behave as if this is the first they've seen this, and that's
what gets me boiling..

Where has anyone said anything like that? They've just said they think
it's bad practice. Just because it's in some Sun/Jakarta/IBM code
doesn't make it good practice.

The point is not to make this a better pattern. It's simply a self
documenting piece of code that insures and guarantees the reader what
the effects of this call are.

But simultaneously confuses the reader about the nature of
Thread.sleep. The way I see it, there are two situations: either the
reader doesn't know about Thread.sleep before they read your code, or
they do. If they do, they'll understand that Thread.sleep or
Thread.currentThread.sleep() do the same thing. If they don't, then
reading Thread.sleep() might send them off to the documentation if they
don't accurately guess the meaning to start with. They can easily find
out what it means. Reading Thread.currentThread().sleep() gives them
the information about what it means *this* time slightly faster, but at
the cost that they then think they know about Thread.sleep when they
don't.

Thank you for the effort; it should've been Cruz though who should do
the work

I suspect it took me less time to do the Google search than it took you
to complain about it though...

 

[Dario]

It *encourages* bugs though. Giving people the idea that a method
operates on whatever object you tell it to when in fact it doesn't is a
fundamentally bad idea, because sooner or later they *will* call it
expecting it to operate on the object they specify which doesn't happen
to be "the right one".
Agreed.


But simultaneously confuses the reader about the nature of
Thread.sleep. The way I see it, there are two situations: either the
reader doesn't know about Thread.sleep before they read your code, or
they do. If they do, they'll understand that Thread.sleep or
Thread.currentThread.sleep() do the same thing. If they don't, then
reading Thread.sleep() might send them off to the documentation if they
don't accurately guess the meaning to start with. They can easily find
out what it means. Reading Thread.currentThread().sleep() gives them
the information about what it means *this* time slightly faster, but at
the cost that they then think they know about Thread.sleep when they
don't.

Agreed.

I completly agree with Jon Skeet.

We (I and my colleagues) always follow this part of the
"Code Conventions for the Java Programming Language"
http://java.sun.com/docs/codeconv/html/CodeConventions.doc9.html#587

Avoid using an object to access a class (static) variable or method.
Use a class name instead. For example:

classMethod(); //OK
AClass.classMethod(); //OK
anObject.classMethod(); //AVOID!

We think these rules are right because they do not make "false
impressions" about a method or a variable.

- Dario

 

[Ricardo]

Jon Skeet wrote:

[snip]

Novice programmer reads your code. He hasn't encountered Thread.sleep
before. He reads the code and thinks, "Ah, Thread.sleep must be an
instance method which sends whatever thread it's called on to sleep."

Later, when he's writing some other code where he (foolishly) wants to
make another thread pause, he calls t.sleep() and then gets confused
when it makes "the wrong thread" sleep.




In your view - not in mine. It gives a false impression of the nature
of the Thread.sleep call, in my view - and that doesn't add to
readability, it reduces it.

You're implying that C is therefore more readable than A. But if someone
*mistakenly* typed the third line as thread.sleep(500), it would then be
the fault of that code in A. That's very interesting..


[snip again]

My design would be for the language to forbid calling
Thread.currentThread().sleep() in the first place (as C# does, btw).
From that starting point, I see nothing particularly wrong in the
design of the Thread class. The "all static methods operate on the
current thread; all instance methods operate on the instance thread"
rule is hardly difficult to learn.

So now the language outght to be changed to accomodate a rather obscure
class.

It would be a good change though.

However, and back the real world, as long as thread.start(),
thread.sleep(500), thread.interrupt(), thread.yield() are all valid and
legal calls, a class with such an interface shall be condemned. We
obviously disagree.

 

[P.Hill]

We (I and my colleagues) always follow this part of the
"Code Conventions for the Java Programming Language"
http://java.sun.com/docs/codeconv/html/CodeConventions.doc9.html#587

"Avoid using an object to access a class (static) variable or method."

Oh don't show him that, for it only says "Avoid" not
"Don't ever", now he's got wiggle room!

-Paul

 

[Ricardo]

It *encourages* bugs though. Giving people the idea that a method
operates on whatever object you tell it to when in fact it doesn't is a
fundamentally bad idea, because sooner or later they *will* call it
expecting it to operate on the object they specify which doesn't happen
to be "the right one".

I don't see how this could be the cause.
Having class methods called sleep and yield in the same class that also
provides instance methods called start and interrupt is the problem.

Where has anyone said anything like that? They've just said they think
it's bad practice. Just because it's in some Sun/Jakarta/IBM code
doesn't make it good practice.

We may be reading those posts differently.

Regardless, it demonstrates that this style has been around for a long
time, and is used by a great number of people, and that is not a symptom
of their ignorance, nor is it intended to fool novice programmers. Some
of the Sun docs, however, go to a much more extreme and dangerous
measure where your case can easily be made, and they *must* make some
changes. See document referenced in response to Carl Howells' post.

I suspect it took me less time to do the Google search than it took you
to complain about it though...

I hope Cruz is taking notes... I tried it. It's really that simple.

 

[Ricardo]

now he's got wiggle room!

Nah.. Just the title of the thread is enough.. :-D

 

[Ricardo]

See document referenced in response to Carl Howells' post.

A link would certainly help

<http://java.sun.com/j2se/1.4.2/docs/guide/misc/threadPrimitiveDeprecation.html>

 

[Jon A. Cruz]

Regardless, it demonstrates that this style has been around for a long
time, and is used by a great number of people, and that is not a symptom
of their ignorance, nor is it intended to fool novice programmers. Some
of the Sun docs, however, go to a much more extreme and dangerous
measure where your case can easily be made, and they *must* make some
changes. See document referenced in response to Carl Howells' post.

I don't dispute that it's bee around for a long time.

However I tend to see that it's just as easily due to the "rouge tile"
or similar bug pattern.

(for those who aren't familiar with it, basically copy-n-paste propoates
a bug or bad code from one place to another)


And just for the record, when at the company where I learned Java, one
senior programmer spent a week walking the halls cussing Gosling up one
side and down the other for some of his actual coding. Steve McConnell's
books also back up that being a good designer and constructing good code
do not always go hand-in-hand.

I hope Cruz is taking notes... I tried it. It's really that simple.

Yes, which is exactly why I didn't bother cluttering my post with it.

 

[Jon A. Cruz]

Have you worked with production source code authored by
Sun, Apache/Jakarta, or even IBM?

Why, yes I have. Quite often. And have had to fix it.

It's generally not the type of thing that is proper to discuss here, but
I'd be glad to fill you in on more details if you privately send me an
email address other than (e-mail address removed)

 

[Jon Skeet]

I don't see how this could be the cause.
Having class methods called sleep and yield in the same class that also
provides instance methods called start and interrupt is the problem.

Not if you make sure they're always called using appropriate syntax, ie

Thread.sleep(...)
someThread.start()

That leads to the *obvious* question on the part of the reader, "Which
thread is being put to sleep?" Now, in my view the obvious answer is
"the current one", but if the answer isn't obvious, the reader can look
at the API. No "false knowledge" is being implied with the above code -
it *is* being implied using your syntax.

We may be reading those posts differently.

Regardless, it demonstrates that this style has been around for a long
time, and is used by a great number of people, and that is not a symptom
of their ignorance, nor is it intended to fool novice programmers.

Are you absolutely *sure* it's not a symptom of ignorance? I suspect at
least *some* of the cases in Jakarta/IBM/Sun code it *is* through
ignorance.

Some of the Sun docs, however, go to a much more extreme and dangerous
measure where your case can easily be made, and they *must* make some
changes. See document referenced in response to Carl Howells' post.

Sorry, I'm not seeing what you're saying here...

 

[Jon Skeet]

You're implying that C is therefore more readable than A.

No, you're inferring that. I'd use B, which I believe is the most
readable.

But if someone
*mistakenly* typed the third line as thread.sleep(500), it would then be
the fault of that code in A. That's very interesting..

What would be the fault of the code in A is someone writing an entirely
different bit of code (which may be nowhere near any code *starting* a
thread) but remembering code A and assuming that they can call
Thread.sleep "on" a thread instance, getting that thread to sleep.

So now the language outght to be changed to accomodate a rather obscure
class.

Well, the language ought never to have been designed that way in the
first place. It's not *just* with Thread.sleep that this comes up, it's
just that that's the most common occurrence. As it is, there's a lot of
code out there which uses such syntax, unfortunately.

The change now could be just for 1.5 and onwards (say) - if you specify
-source 1.5 it could turn it into an error then, leaving old code
working the same way if you don't specify the -source option.

It would be a good change though.

However, and back the real world, as long as thread.start(),
thread.sleep(500), thread.interrupt(), thread.yield() are all valid and
legal calls, a class with such an interface shall be condemned. We
obviously disagree.

We certainly do. I find it odd that you believe it would be a good
change to make what you consider a good practice to be an error. I
prefer to treat it as an error already (in Eclipse) because I consider
it a *bad* practice.