Wednesday, March 26, 2008

Bug bite

It's a very subtle bug but results in great headache. I didn't notice my bug until someone pointed it out. Good lesson learned: Always have tests when you refactor.

See if you could spot it.


// BEFORE
private LockID[] getAllLockIDs(LockID id) {
LockID[] lids = new LockID[transactionStack.size() + 1];
lids[0] = id;
for (int i = 1; i < lids.length; i++) {
TransactionContext tc = (TransactionContext) transactionStack.get(i - 1);
lids[i] = tc.getLockID();
}
return lids;
}


// AFTER
private List getAllLockIDs(LockID id) {
List lids = new ArrayList();
lids.add(id);
for (int i = 1; i < transactionStack.size(); i++) {
TransactionContext tc = (TransactionContext) transactionStack.get(i - 1);
lids.add(tc.getLockID());
}
return lids;
}

3 comments:

Dennis Nguyen said...

you missed the last element of transactionStack

Niv said...

In the first case, lids has an extra element.
If transactionStack.size() is 5,
lids is of size 6.
In the previous code,change to
lids = new LockID[transcationStack.size()];

Unknown said...

Hi Hung,

I thinks, in the last code, you missed the last element of transaction stack, you must start index from 0 to transactionStack.size and you make sure that transactionStack is not null.
The seconds problem when you using the array is you must using implicit casting from the object to LockID when you using them.

the code is modified:
// AFTER
private List getAllLockIDs(LockID id) {
List lids = new ArrayList();
lids.add(id);
for (int i = 0; i < transactionStack.size(); i++) {
TransactionContext tc = (TransactionContext) transactionStack.get(i);
lids.add(tc.getLockID());
}
return lids;
}

or
// AFTER
private List getAllLockIDs(LockID id) {
List lids = new ArrayList();
lids.add(id);
for (int i = 1; i < transactionStack.size() + 1; i++) {
TransactionContext tc = (TransactionContext) transactionStack.get(i - 1);
lids.add(tc.getLockID());
}
return lids;
}