Re: Anybody want to do a Java code review?
A number of comments after a quick glance:
For all your collections you used a linked list, why? Do you need to do alot of removes? If not, an ArrayList is probably better.
Also, every time you add something you first check if it is there already, if so why not use a Set which doesn't allow duplicates?
In your collection find methods you have the following code:
foundit:
while(i.hasNext())
{
theCurrent = (CustomerPO)i.next();
if (theCustomer == theCurrent.getCustomerID())
{
if (POID == theCurrent.getCustomerPONumber())
{
break foundit;
}
}
}
if (i.hasNext())
{
return theCurrent;
}
else
{
return null;
}
A number of questions/comments:
1. Why do you need the break, once you have found the entry why not just return it?
2. What happens if the element that you find is the last one, i.hasNext() will be false but you still found the element.
You synchronized alot of methods, but not all of them, for example, you synchronized all the setAddress methods but none of the setPerson methods, do you expect to have many threads updating an address at the same time? If you do the synchronization might not help, take the following example: both Thread 1 and Thread 2 want to update Street Address 1 and 2. Thread 1 updates Address 1 then goes to sleep thread 2 updates Address 1 (Thread 1 is finished) and Address 2. Thread 1 then updates Address 2 (because thread 2 finished). The end result is that Address 1 has the new address that thread 2 entered and address 2 has the new address that thread 1 entered, in short the address is not consistent.
I don't really see where all the pieces fit together, what will drive the system?
