pivot-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sandro Martini <sandro.mart...@gmail.com>
Subject Re: FindBugs Analysis
Date Mon, 20 Jun 2011 12:51:41 GMT

>Keep in mind that the return value of doPut() is not a status - it
indicates whether a resource was created or not. Status codes (specifically
errors) are returned by throwing an exception.
>It's not entirely clear to me what the return value of File#delete() means.
Did an error occur, or did the file simply not exist to begin with? If the
latter, I think the code is probably fine as-is. If the former, we should
probably check the return value and throw a QueryException if false.
Note that file#delete will not throw an error if the delete is not possible,
take a look here for example:
So wouldn't be good to have the doDelete (indicating if the resource has
been deleted or not, like in the doPut) return a boolean:
            created = doPut(path, value);
so we could have
            deleted = doDelete(path, value);
and behave like in the other method ...
but maybe for the 2.1 to not break here method signature, ok ?

>> wtk/src/org/apache/pivot/wtk/content/TableViewDateCellRenderer.java:34
>> Found static field of type java.text.DateFormat
>Why is this a problem? (FYI, as I recall, you made this change)
I remember, I put it static for performance improvement, but it say that
could not be safer in case of multi-thread usage (and I don't think it's our
case), but I'll see if there is a way to keep it static but safer ...

>> And last:
>> tests/src/org/apache/pivot/tests/ShutdownTest.java:55
>> Unchecked/unconfirmed
>> cast from org.apache.pivot.wtk.Dialog to org.apache.pivot.wtk.Alert
>Ignore all these. The casts are valid.
Ok, but I guess if for the 2.1 could make sense to change a little their
generics signature to have compile-time checks for these like <Y extends X>,
but it's a small thing ...

I'll make other small changes as soon as possible, thank you for the review
and sorry for duplicate old ones (you know, my little memory).


View this message in context: http://apache-pivot-developers.417237.n3.nabble.com/FindBugs-Analysis-tp3076403p3086056.html
Sent from the Apache Pivot - Developers mailing list archive at Nabble.com.

View raw message