Whats wrong with this code....

I

is0871

public List loadNotes(Long id)
{
List notes = new ArrayList() ;
try
{
notes = (List) someLocalHome.findByPrimaryKey(id) ;
if (notes != null)
{
Iterator i = notes.iterator() ;
while (i.hasNext())
{
DummyBean bean = new DummyBean( (DummyLocal) i.next()) ;
System.out.println("found bean: " + bean) ;
notes.add(bean) ;
}
}
else
{
System.out.println("Could not find any notes!") ;
}
}
catch(Exception e)
{
e.printStackTrace() ;
}

return notes ;
}


If you can figure out whats wrong in the above code(wihout compiling),
then you're the kind of person we're looking for. If interested in
working in a famous special effects house then apply online here:
http://www.ilm.com/jobs/it/20050104.html
 
J

John McGrath

If you can figure out whats wrong in the above code(wihout compiling)

One obvious problem is that you are iterating over a collection and
adding to the same collection in the process. This will result in a
ConcurrentModificationException.
 
I

iamfractal

public List loadNotes(Long id)
{
Snipppp.

working in a famous special effects house then apply online here:
http://www.ilm.com/jobs/it/20050104.html

Hideously off-topic:

ILM have really lost the plot of late.

It's almost as though they've stopped striving for realism and devoted
themselves to computer-realism. Everything's so plasticky. So
playstationy. So perfectly unworldly. Ok, stop-motion looks pathetic
in retrospect, but at least they moved on. Now they seem contented
with fatter address busses. Who knows, maybe with 1000gigs of RAM they
can finally have Anakin looking directly at Jar-Jar rather than two
feet to his left.

..ed

www.EdmundKirwan.com - Home of The Fractal Class Composition.
 
H

HK

public List loadNotes(Long id)
{
List notes = new ArrayList() ;
try
{
notes = (List) someLocalHome.findByPrimaryKey(id) ;
[etc.]

The code was intentionally convoluted by putting all
the open braces in the wrong places.

Trolling on,
Harald.-)
 
M

Matt Humphrey

John McGrath said:
One obvious problem is that you are iterating over a collection and
adding to the same collection in the process. This will result in a
ConcurrentModificationException.

You're quite right that adding to a collection while iterating over it is
usually a bad idea, although I don't think it's possible to tell exactly
what will happen because we don't know the actual class for either the
iterator or the collection. Aside from compilation errors, without knowing
what the fragment is supposed to do it's impossible to say how it might be
wrong.

Cheers,
Matt Humphrey (e-mail address removed) http://www.iviz.com/
 
T

Thomas Weidenfeller

Matt said:
Aside from compilation errors, without knowing
what the fragment is supposed to do it's impossible to say how it might be
wrong.

There is one obvious thing:

But it is to obvious ...

/Thomas
 
M

Matt Humphrey

Thomas Weidenfeller said:
There is one obvious thing:


But it is to obvious ...

I had also thought that the newly created list is spurrious because it
appears to always be overwritten, but there is a case where the method
returns the empty list. If the findByPrimaryKey throws an exception, the
method will return the empty list, rather than null because the exception is
caught. Otherwise, note is either some kind of List or null. It appears to
me more as plain bad design rather than being formally incorrect. (What's
it *supposed* to return if findByPrimaryKey fails?)

Cheers,
Matt Humphrey (e-mail address removed) http://www.iviz.com/
 
T

Thomas Weidenfeller

Matt said:
It appears to
me more as plain bad design rather than being formally incorrect.

I really don't know. The else part indicates that the calling method
should better be prepared to get a null returned, because that's what
will happen in that case. But it is all twisted. If there should be a
distinction between a failure and a "no results found", I would have
returned an empty list for "no results found", and the null for the
failure (or the exception for the failure). That code does it the other
way around.

I think we see the typical problems with such examples. Without knowing
what it is supposed to do, everything can be interpreted in different
ways. Hey, that's the idea!

I hereby declare the the problem is the name of the method. I bet the
developer wanted to call it noteLoading() instead of loadNotes() :)

/Thomas
 
V

Virgil Green

public List loadNotes(Long id)
{
List notes = new ArrayList() ;
try
{
notes = (List) someLocalHome.findByPrimaryKey(id) ;
if (notes != null)
{
Iterator i = notes.iterator() ;
while (i.hasNext())
{
DummyBean bean = new DummyBean( (DummyLocal) i.next()) ;
System.out.println("found bean: " + bean) ;
notes.add(bean) ;
}
}
else
{
System.out.println("Could not find any notes!") ;
}
}
catch(Exception e)
{
e.printStackTrace() ;
}

return notes ;
}


If you can figure out whats wrong in the above code(wihout compiling),
then you're the kind of person we're looking for. If interested in
working in a famous special effects house then apply online here:
http://www.ilm.com/jobs/it/20050104.html

That's the worst attempt at Fortran that I've ever seen.
 
J

John McGrath

I don't think it's possible to tell exactly what will happen because we
don't know the actual class for either the iterator or the collection.

Good point. There is no guarantee that Iterators will be fail-fast.
 

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

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top