On Sat, Nov 22, 2008 at 9:19 AM, sebb <[EMAIL PROTECTED]> wrote:
> Just thought to run Findbugs on the code.
>
<snip/>
Thanks for going over the Findbugs list. While this is a bit of a déjà
vu for me ...
http://markmail.org/message/si5kphud52ntxbqi
... I don't expect others to remember the discussion :-) and should
have commented on it earlier in this thread.
There is one new finding, I'll comment on it below.
> There are a lot of cases of the statement:
>
> private Log appLog = LogFactory.getLog(...)
>
> which appear in serializable classes.
>
> However Log does not appear to be Serializable, so this will cause a
> problem if the classes are serialised. So long as the Log fields are
> private, they could just be made static and final.
>
> ==
>
> org.apache.commons.scxml.io.SCXMLDigester.digest() has:
> if (scxml != null) {
> ModelUpdater.updateSCXML(scxml);
> }
> scxml.setLegacy(true); // could be null - should be in
> previous if statement
>
> ==
<snap/>
Indeed :-) I'll fix that in a minute, but I won't be cutting a new RC
just for this:
1) We could do better than a NPE, but its a fatal parsing error at that point
2) Its a long deprecated class (deprecated in 0.7)
-Rahul
>
> Class org.apache.commons.scxml.model.Data defines non-transient
> non-serializable instance field node
>
> ==
>
> org.apache.commons.scxml.env.jexl.JexlEvaluator.evalCond(Context,
> String) has Boolean return type and returns explicit null
>
> org.apache.commons.scxml.env.jsp.ELEvaluator.evalCond(Context, String)
> has Boolean return type and returns explicit null
>
> These will cause problems if used later with autoboxing.
>
> ==
>
> Inconsistent synchronization of
> org.apache.commons.scxml.SCXMLExecutor.errorReporter; locked 76% of
> time
>
> Similarly for eventDispatcher, stateMachine, superStep in the same class
>
> It appears that the class will be used in multiple threads, so if
> those variables can be accessed from multiple threads, then all access
> to them will need to be synchronized.
>
> ==
>
> There are a few other Findbugs warnings - mainly in the test code. Let
> me know if you want the full list (though if you use Eclipse, it's
> probably easier to add the Findbugs plugin).
>
> ==
>
> I also just noticed that there are a few instance fields that could be
> made final - e.g. most of the items set up in the
> org.apache.commons.scxml.SCInstance.SCInstance constructor.
> This would improve thread-safety.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
