Signs of stupid Java code

D

Darryl L. Pierce

Tor said:
Why do you think so?

In my experience, such a situation would require a no-arg overloaded version
of the method.
Unless you don't like generalized examples and
have little experience with the situation yourself.

I have plenty of experience. My experience is that in a case where you need
to invoke a method knowing in advance that you're passing no object, you
should instead be invoking a method with no arguments. If you have 2
overloaded methods [foo(String) and foo(Object)] but need to invoke foo()
with the prior knowledge that you won't be passing an object, then the
proper design would include a foo() method, rather than casting null.
Or do you never
ever pass null as a parameter to a method?

Yes, I do. But, that has nothing to do with *casting* null to some type.
Are you perhaps opposed to
polymorphism and instead would call the methods fooObject() and
fooString() respectively?

Now you've moved past what I said into the area of strawman arguments.
A concrete example, then: JDialog's constructors with no parent, you
need to be explicit whether that's no parent Dialog or no parent
Frame.

Sun's not infallible. Though, it's not a case of Sun giving the wrong
constructors: it's the user's *use* of the constructor that's wrong, if the
user's instantiating JDialog with:

JDialog dialog = new JDialog((Frame )null,true);

rather than using:

JDialog dialog = new JDialog();

dialog.setModel(true);

Or maybe Sun *was* dumb for not supplying constructors such as:

JDialog(boolean modal);
JDialog(boolean model,String title);
JDialog(String title);

After all, if it's acceptable to *not* have a Frame or Dialog as a parent,
then it should be possible to construct the object *without* specifying
those objects, right?
 
D

Darryl L. Pierce

Thomas said:
Was I clear? Maybe not, if not then I'm sorry.

No, the problem seems that *I* wasn't clear. I had a very specific example
and I've now had no less than three people tell me that I'm wrong, but not
a one was addressing what I *wrote* but instead what they *read*, which
were two different things.

I didn't say, for example, that casting null to a type is wrong. On the
contrary, I said that casting null to a type in order to invoke an
overloaded method is dumb in that, if there's a case where the method can
be invoked without an object being passed, then there should be a no-arg
overloaded version of the method. That's why we *have* such things as
overloading.
The point I was making was that code like that is /not/ an example of bad
code, and is a perfectly acceptable practice. I've only seen the need for
it 4 times in 8 years, but there is definately a need for it.

You may be seeing many cases where the casts are not warranted,
particularly
by folks who don't understand polymorphism. But you were wrong when you
said that the example:

foo((Object)null);

was very contrived. It isn't.

Then perhaps "contrived" was the wrong word. However, it's still an example
of bad code. If you have to explicitly cast null to a type, then there's a
design flaw in the choice of overloaded methods.
 
B

Bent C Dalager

I didn't say, for example, that casting null to a type is wrong. On the
contrary, I said that casting null to a type in order to invoke an
overloaded method is dumb in that, if there's a case where the method can
be invoked without an object being passed, then there should be a no-arg
overloaded version of the method. That's why we *have* such things as
overloading.

The dumbness in this case rests not with the person who wrote the
cast, but with the person who designed the library which requires you
to do that cast.

Cheers
Bent D
 
J

John C. Bollinger

Jim said:


Ok since I didn't see it in newbie

int MAX_SIZE = 10;
int [] data = new int [MAX_SIZE];

// Later

for(int i = 0;i < MAX_SIZE;i++)

I've seen this several times.

Is the problem that MAX_SIZE is not declared static? I would call that
a coding mistake, not a sign of stupidity. (And, depending on the
context, perhaps not even a mistake.)

On the other hand, it is recognized in many quarters as good programming
practice to use symbolic names for constants other than zero and +/-
one. IIRC, it is even included in Sun's Java coding conventions. Doing
so makes it easy to change all copies at once if a change is ever
necessary, and it clears up the question of whether this "10" over here
is coincidentally the same number as that "10" over there, or whether
the algorithm in fact depends on them being the same number. If the
constant is well named then it even clarifies the code by giving the
reader a clue as to the significance of the constant wherever it appears.


John Bollinger
(e-mail address removed)
 
D

Darryl L. Pierce

Bent said:
The dumbness in this case rests not with the person who wrote the
cast, but with the person who designed the library which requires you
to do that cast.

Good point. A double-layered example, then. :)
 
J

Joona I Palaste

John C. Bollinger said:
Jim said:
Ok since I didn't see it in newbie

int MAX_SIZE = 10;
int [] data = new int [MAX_SIZE];

// Later

for(int i = 0;i < MAX_SIZE;i++)

I've seen this several times.
Is the problem that MAX_SIZE is not declared static? I would call that
a coding mistake, not a sign of stupidity. (And, depending on the
context, perhaps not even a mistake.)
On the other hand, it is recognized in many quarters as good programming
practice to use symbolic names for constants other than zero and +/-
one. IIRC, it is even included in Sun's Java coding conventions. Doing
so makes it easy to change all copies at once if a change is ever
necessary, and it clears up the question of whether this "10" over here
is coincidentally the same number as that "10" over there, or whether
the algorithm in fact depends on them being the same number. If the
constant is well named then it even clarifies the code by giving the
reader a clue as to the significance of the constant wherever it appears.

I was also wondering where the stupidity would be. Certainly not in
using MAX_SIZE instead of 10 - I count that as cleverness. Perhaps
it's in using MAX_SIZE instead of data.length?
 
J

Joona I Palaste

Joona I Palaste said:
Can you think of examples of Java code that are signs of programmer
stupidity? I'll start:
synchronized (new Object()) {
/* ... */
}
The synchronisation is utterly pointless in this case.

Here's an example of real-life stupidity. One of my coworkers just wrote
a singleton class (complete with a private default constructor and no
public constructors). Then he made the getInstance() method dynamic
instead of static. Great! How am I supposed to get the singleton
instance then? By already having it?
 
J

Joona I Palaste


That tought occurred to me as well, but besides being a contrived
solution, doesn't that defeat the purpose of a singleton? Or should I
make the subclass a singleton as well?
I solved the problem by making getInstance() static.
 
J

Jay O'Connor

Joona said:
Jim wrote:

Ok since I didn't see it in newbie

int MAX_SIZE = 10;
int [] data = new int [MAX_SIZE];

// Later

for(int i = 0;i < MAX_SIZE;i++)

I've seen this several times.
I was also wondering where the stupidity would be. Certainly not in
using MAX_SIZE instead of 10 - I count that as cleverness. Perhaps
it's in using MAX_SIZE instead of data.length?
The problem , as I understand it, is that between "int MAX_SIZE = 10;"
and the for loop, or possible even *in* the for loop, MAX_SIZE can be
changed, which would not be good.
 
H

Harald Hein

Joona I Palaste said:
That tought occurred to me as well, but besides being a contrived
solution, doesn't that defeat the purpose of a singleton? Or should I
make the subclass a singleton as well?

You should have given your colleauge a severe beating.

Harald
 
J

Joona I Palaste

You should have given your colleauge a severe beating.

First, we work in separate buildings (with a few km in between), second,
I think it was accidental, he meant it to be static.
 
D

Doug Pardee

Joona I Palaste said:
synchronized (new Object()) {
/* ... */
}

The synchronisation is utterly pointless in this case.

Not quite. ALMOST pointless, but not "utterly pointless".

Recall that synchronization in Java involves two different things:
1) mutual exclusion
2) memory barriers

The code shown does indeed nullify the mutual exclusion. However, the
two memory barriers still exist, and the synchronized block is still
located between them.
 
J

Jim

Joona said:
Jim wrote:


Ok since I didn't see it in newbie

int MAX_SIZE = 10;
int [] data = new int [MAX_SIZE];

// Later

for(int i = 0;i < MAX_SIZE;i++)

I've seen this several times.
I was also wondering where the stupidity would be. Certainly not in
using MAX_SIZE instead of 10 - I count that as cleverness. Perhaps
it's in using MAX_SIZE instead of data.length?
The problem , as I understand it, is that between "int MAX_SIZE = 10;"
and the for loop, or possible even *in* the for loop, MAX_SIZE can be
changed, which would not be good.

Yes, MAX_SIZE can be changed, but even more important so
can the array 'data'! (Reason for the Later remark)

For instance what would happen if someone reassigned data to say

data = new int[] {4,5,6};

Rule : Always use the 'length'

Jim
 
M

Michael Borgwardt

Jim said:
Yes, MAX_SIZE can be changed, but even more important so
can the array 'data'! (Reason for the Later remark)

For instance what would happen if someone reassigned data to say

data = new int[] {4,5,6};

Rule : Always use the 'length'

Actually, I'd say it depends. If the intention was for the Array to be
initialized with MAX_SIZE and *not* changed later on, then the
reassignment might be a serious mistake and an early discovery through
an ArrayIndexOutOfBoundsException preferable to working with erroneous
data.
 
B

Ben_

Actually, I'd say it depends. If the intention was for the Array to be
initialized with MAX_SIZE and *not* changed later on, then the

Isn't 'final' the right keyword then ?
reassignment might be a serious mistake and an early discovery through
an ArrayIndexOutOfBoundsException preferable to working with erroneous
data.

Changing a 'final' variable is detected at compile time, so the problem is
spotted even earlier.
 
C

Chris Uppal

Joona said:
First, we work in separate buildings (with a few km in between), second,
I think it was accidental, he meant it to be static.

His testing strategy is obviously not over-elaborate...

-- chris
 
A

Andrea Desole

just taken from a project I am unfortunately working with :-((

if(text.charAt(i)=='0'||text.charAt(i)=='1'||text.charAt(i)=='2'||text.charAt(i)=='3'||text.charAt(i)=='4'||text.charAt(i)=='5'||text.charAt(i)=='6'||text.charAt(i)=='7'||text.charAt(i)=='8'||text.charAt(i)=='9'||text.charAt(i)=='-')
{}
else
{
return false;
}
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,780
Messages
2,569,611
Members
45,260
Latest member
kentcasinohelpWilhemina

Latest Threads

Top