FindBugs

[Andrey]

Hi all,

I have tried to apply some recommendations of FindBug and PMD. No real issue fixed but some performance recommendations make sense.

You can have a look in this clone:

http://code.google.com/r/py4fun-findbugs/source/list

If I do not get any objections this week, I will apply the changes. The FindBugs and PMD reports will be available under a separate Maven profile.

-

Andrey

[Jordan Angold]

These changes look mostly okay, but I did spot some things I found
questionable:

1.
http://code.google.com/r/py4fun-findbugs/source/diff?spec=svnf8c8accab5f822bee606a20c0ca1d9490208b2fb&r=f8c8accab5f822bee606a20c0ca1d9490208b2fb&format=side&path=/src/main/java/org/yaml/snakeyaml/emitter/Emitter.java
In checkEmptyDocument(), why not use "e.getValue().length() == 0" ?

2. In Emitter, why not just:
ESCAPE_REPLACEMENTS.put ( '\0', "0" );
instead of using Character.valueOf() every time. Autoboxing uses
valueOf() automatically, so the code is equivalent, just cleaner.

3.
http://code.google.com/r/py4fun-findbugs/source/diff?spec=svnd40931d25a6e75e5c24dcc82504d6f2522ee5cf8&r=d40931d25a6e75e5c24dcc82504d6f2522ee5cf8&format=side&path=/src/main/java/org/yaml/snakeyaml/nodes/NodeTuple.java
In NodeTuple, why are the two fields no longer final? Keeping them
final will prevent anyone from accidentally writing code that changes
them.

4.
http://code.google.com/r/py4fun-findbugs/source/diff?spec=svnd40931d25a6e75e5c24dcc82504d6f2522ee5cf8&r=d40931d25a6e75e5c24dcc82504d6f2522ee5cf8&format=side&path=/src/main/java/org/yaml/snakeyaml/extensions/compactnotation/PackageCompactConstructor.java
I'm not sure why this change was made. Can you explain it? It looks
like code is duplicated.

Other than that, the changes look sensible.

/Jordan

[Andrey]

1. Applied

2. Applied

The tools say that new Character() is no efficient because it always creates an object, while Character.valueOf() get the value from the cache for first 256 characters. Since we use a static data structure the performance is not important at all. Readability is more important

3. The tools say that final class does not need to have final modifiers for variables - the compiler will check it anyway. They were removed because they were redundant

4. Applied (reverted)

The tools say that to have an empty catch block is a bad practice (and I agree). Unfortunately the Java API does not allow to check whether a class is available without throwing an exception.

Thank you,

Andrey