> I have a "Card" class which has objects that represent cards out of a
> standard poker deck. I statically initialize the array.
>
> My basic question is: Are there circumstances in which the following
> code will not produce "==" comparable Card objects? These objects may
> be passed through RMI or otherwise serialized.
I believe, malicious code could grab a deserialised instance before
readResolve, but other than that I think you have covered all the bases.
> import java.io.InvalidObjectException;
> import java.io.ObjectStreamException;
>
> public final class Card {
> static final long serialVersionUID = 1L;
You mean:
public final class Card implements java.io.Serializable {
private static final long serialVersionUID = 1L;
> public final transient Suit suit;
> public final transient String number;
> public final transient int value;
I would calculate these on demand.
> protected final Object clone() throws CloneNotSupportedException {
> throw new CloneNotSupportedException();
> }
The class is final, so why bother?
> protected final Object readResolve() throws ObjectStreamException {
Why protected?
Tom Hawtin
Daniel Pitts - 25 Mar 2007 23:18 GMT
> > I have a "Card" class which has objects that represent cards out of a
> > standard poker deck. I statically initialize the array.
[quoted text clipped - 5 lines]
> I believe, malicious code could grab a deserialised instance before
> readResolve, but other than that I think you have covered all the bases.
I'm not worried about malicious code in this place. the value only
"really" matters on a server side, which shouldn't load or execute
untrusted code at all. But out of curiosity, how WOULD malicious code
obtain a pre-readResolved reference? It doesn't matter in this case,
but if its possible, I should learn about it :-)
> > import java.io.InvalidObjectException;
> > import java.io.ObjectStreamException;
[quoted text clipped - 6 lines]
> public final class Card implements java.io.Serializable {
> private static final long serialVersionUID = 1L;
Yes I did.
> > public final transient Suit suit;
> > public final transient String number;
> > public final transient int value;
>
> I would calculate these on demand.
Perhaps, but I'm avoiding getters here, and there seems little reason
to not pre-calculate these values.
> > protected final Object clone() throws CloneNotSupportedException {
> > throw new CloneNotSupportedException();
> > }
>
> The class is final, so why bother?
Why bother? Just in case :-) Defensive programmming...
Having it declared this way lets future developers know it should not
be changed. This is actually noted in a comment that I stripped out.
It is also stolen from java.lang.Enum;
> > protected final Object readResolve() throws ObjectStreamException {
>
> Why protected?
Well, my IDE complains about unused private methods, and public seems
a bad idea, as well as package-protected.
Anyway, other than malicious readResolve interceptors, this looks like
it would be safe and correct code?
Thanks,
Daniel.
Piotr Kobzda - 26 Mar 2007 11:50 GMT
>>> protected final Object readResolve() throws ObjectStreamException {
>> Why protected?
>
> Well, my IDE complains about unused private methods, and public seems
> a bad idea, as well as package-protected.
Using protected is equally bed idea as using package-protected... You
should use @SuppressWarnings("unused") to calm down the IDE.
piotr
Chris Uppal - 26 Mar 2007 08:19 GMT
> > public final transient Suit suit;
> > public final transient String number;
> > public final transient int value;
>
> I would calculate these on demand.
Why ? If you make them non-final then they aren't threadsafe.
-- chris
Tom Hawtin - 26 Mar 2007 11:54 GMT
>>> public final transient Suit suit;
>>> public final transient String number;
>>> public final transient int value;
>> I would calculate these on demand.
>
> Why ? If you make them non-final then they aren't threadsafe.
I meant, I would calculate the values every time. I wouldn't have them
as fields at all. So, for instance, there would be a method getNumber:
public String getNumber() {
return numbers[ordinal % CARDS_PER_SUIT];
}
Certainly lazy initialisation is usually not a winner. However, in this
case it would not actually remove thread-safety for the same reason
String.hash works.
Tom Hawtin
Chris Uppal - 26 Mar 2007 12:23 GMT
> > > > public final transient Suit suit;
> > > > public final transient String number;
[quoted text clipped - 5 lines]
> I meant, I would calculate the values every time. I wouldn't have them
> as fields at all.
Ah, I see. A valid design choice...
> Certainly lazy initialisation is usually not a winner. However, in this
> case it would not actually remove thread-safety for the same reason
> String.hash works.
Point taken, and agreed.
-- chris
Daniel Pitts - 27 Mar 2007 16:18 GMT
On Mar 26, 4:23 am, "Chris Uppal" <chris.up...@metagnostic.REMOVE-
THIS.org> wrote:
> > > > > public final transient Suit suit;
> > > > > public final transient String number;
[quoted text clipped - 15 lines]
>
> -- chris
It may not remove thread safety, but I think it actually complicates
the design. I think my original class is simplified by having a few
precalculated values and a clear process which calculates them.
Hendrik Maryns - 27 Mar 2007 10:53 GMT
Tom Hawtin schreef:
>>>> public final transient Suit suit;
>>>> public final transient String number;
[quoted text clipped - 11 lines]
>
> Certainly lazy initialisation is usually not a winner.
Can you elaborate on this a bit? Is it only with respect to
multithreading, or in general?
H.
- --
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
http://aouw.org
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html
Tom Hawtin - 27 Mar 2007 11:29 GMT
> Tom Hawtin schreef:
>>
>> Certainly lazy initialisation is usually not a winner.
>
> Can you elaborate on this a bit? Is it only with respect to
> multithreading, or in general?
In general.
It can be a win, but the circumstances are relatively unusual. The
computation needs to be significant in terms of CPU cycles or memory,
and also it needs to be unnecessary to evaluate more often than not.
Lazy initialisation can lose in a number of ways:
* As you say, overhead in dealing with multiple threads.
* The code becomes more complicated and therefore more difficult to
understand and very much more likely to be buggy.
* Checking for initialisation tends to make the code slower. It's not
just the extra check, but the amount of code potentially runnable within
performance critical sections is also massively increased. Big sections
are code are very difficult to optimise, particularly in a system that
compiles at runtime.
* If the initialisation is done always, or almost always, then you are
not gaining anything anyway.
It's one of those techniques that sounds really cool, but actually is
rarely useful. The result is that it tends to get used inappropriately.
Tom Hawtin