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
Hi,

>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:
http://www.exampledepot.com/egs/java.io/deletefile.html
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).

Bye,
Sandro


--
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.

Mime
View raw message