Code Quality in Ruby and Java

Code Quality
What is this s**t?

Steve Hayes - Cogent Consulting Pty Ltd
http://www.cogentconsulting.com.au
steve.hayes@cogentconsulting.com.au

Friday, 28 August 2009 1

In 1990 there were 120 billion lines of code


In 2000 there were 250 billion lines of code


In 2010 there will be 500 billion lines of code


Printed in a font 1mm high
the code listing would stretch
from the Earth to the Moon
and beyond


Code Quality


Code quality is subjective - it requires human
judgment


There have been, and always will be, arguments
about code quality


WTF implies lack of clarity


“Simplicity, clarity, singleness: These are the
attributes that give our lives power and vividness
and joy as they are also the marks of great art.
The seem to be the purpose of God for his
whole creation.”

Richard Holloway


Clear code is easier to understand, easier to
maintain, and easier to extend


There are many things that can reduce the clarity
of code, including:

• inconsistent style
• long methods
• lots of methods in a single class
• repeated code
• methods with many alternative paths


Your particular bugbears may be different


Why metrics?


We substitute metrics for human judgement


People are expensive, CPU time is cheap


Use automation to detect clarity problems, such
as:

• inconsistent style
• long methods
• lots of methods in a single class
• repeated code
• methods with many alternative paths


Use exception based reporting so that people can
focus their energy


on the quality exceptions


or on completely different things


Code Analysis


Let’s start with some Ruby

Formtastic by Justin French


Then we’ll move on to some Java


Duplication


Duplication makes everything else worse


Initial analysis of Formtastic


37 Roodi problems

95 Flog problems


Simian
http://www.redhillconsulting.com.au/products/simian/


Simian - Similarity Analyser | Duplicate Code Detection for the Enterprise 14/08/09 3:41 AM

Simian - Similarity Analyser
Purpose
Simian (Similarity Analyser) identifies duplication in Java, C#, C, C++, COBOL, Ruby, JSP, ASP,
HTML, XML, Visual Basic, Groovy source code and even plain text files. In fact, simian can be used on
any human readable files such as ini files, deployment descriptors, you name it.

Especially on large enterprise projects, it can be difficult for any one developer to keep track of all the
features (classes, methods, etc.) of the system.

Simian runs natively in any .NET 1.1 or higher supported environment and on any Java 1.4 or higher
virtual machine, meaning Simian can be run on just about any hardware and any operating system you
can hope for. Both the Java and .NET runtimes are included as part of the distribution.

Simian can be used as part of the build process during development or as a guide when re-factoring.
Think of Simian as an independent pair of eyes that will assist in raising the quality of your software.

Within minutes, Simian can save you literally thousands of dollars in time spent performing maintenence,
debugging and re-factoring.

With licensing available to suite personal, project and enterprise use, simian is ideally suited for use on
your project.

Why do I need Simian?
Imagine for example that a bug is discovered in a method somewhere in a project. The developer duly
writes a test case, makes the necessary code changes, ensures the test passes, checks the code in and
considers the job finished!

Right?

Simian highlights obvious duplication


Similarity Analyser 2.2.24 - http://www.redhillconsulting.com.au/products/simian/index.html
Copyright (c) 2003-08 RedHill Consulting Pty. Ltd. All rights reserved.
Simian is not free unless used solely for non-commercial or evaluation purposes.
{failOnDuplication=true, ignoreCharacterCase=true, ignoreCurlyBraces=true, ignoreIdentifierCase=true,
ignoreModifiers=true, ignoreStringCase=true, threshold=6}
Found 7 duplicate lines in the following files:
Between lines 949 and 962 in /Users/stevehayes/projects/third-party/formtastic/spec/formtastic_spec.rb
Found 50 duplicate lines in 6 blocks in 1 files
Processed a total of 2066 significant (4341 raw) lines in 6 files
Processing time: 0.144sec


Flay
http://ruby.sadi.st/Flay.html


Confessions of a Ruby Sadist sudo gem install flay 14/08/09 3:46 AM

Ruby Sadist Flog Flay Heckle Kittens About Us

Confessions of a Ruby Sadist
sudo gem install flay

Flay analyzes ruby code for
structural similarities. Differences
in literal values, names,
whitespace, and programming
style are all ignored.

Code that flay reports as similar
is a good candidate for
refactoring.

Try this:

% sudo gem install
flay
% flay lib/*.rb

and see what you find.


Flay detects more subtle duplication


Total score (lower is better) = 2637 8) Similar code found in :iter (mass = 96)
spec/formtastic_spec.rb:1825
1) Similar code found in :iter (mass = 342) spec/formtastic_spec.rb:1844
spec/formtastic_spec.rb:1570 spec/formtastic_spec.rb:1877
2) Similar code found in :iter (mass = 136)
spec/formtastic_spec.rb:1183 9) Similar code found in :iter (mass = 92)
10) Similar code found in :block (mass = 84)
3) IDENTICAL code found in :iter (mass*2 = 136) spec/formtastic_spec.rb:1900
11) IDENTICAL code found in :block (mass*2 = 84)
6) Similar code found in :iter (mass = 100) spec/formtastic_spec.rb:1208

7) Similar code found in :iter (mass = 98) 14) Similar code found in :iter (mass = 75)


So what did I do?


move lines around

change similar code to be the same

changed in-line constants to variables


these are often non-trivial changes


very little change in overall ﬁle length

(3013 => 2897 lines)


How did that change complexity?


37 => 37 Roodi problems

95 => 38 Flog problems


Flog change indicates that methods have
become simpler


Exacerbation


What else exacerbates problems?


Long classes and methods


http://farm4.static.ﬂickr.com/3060/2426522019_7e696fb9af.jpg


Tools that detect long classes and methods are
simple but useful


Roodi
Ruby Object Oriented Design Inferometer
http://github.com/martinjandrews/roodi/tree/master


General Purpose Code Quality


Roodi checks

AssignmentInConditionalCheck Check for an assignment inside a conditional. It‘s probably a mistaken equality comparison.

CaseMissingElseCheck Check that case statements have an else statement so that all cases are covered.

ClassLineCountCheck Check that the number of lines in a class is below the threshold.

ClassNameCheck Check that class names match convention.

CyclomaticComplexityBlockCheck Check that the cyclomatic complexity of all blocks is below the threshold.

CyclomaticComplexityMethodCheck Check that the cyclomatic complexity of all methods is below the threshold.

EmptyRescueBodyCheck Check that there are no empty rescue blocks.

ForLoopCheck Check that for loops aren‘t used (Use Enumerable.each instead)

MethodLineCountCheck Check that the number of lines in a method is below the threshold.

MethodNameCheck Check that method names match convention.

ModuleLineCountCheck Check that the number of lines in a module is below the threshold.

ModuleNameCheck Check that module names match convention.

ParameterNumberCheck Check that the number of parameters on a method is below the threshold.


Roodi parameters

AssignmentInConditionalCheck

CaseMissingElseCheck

ClassLineCountCheck 300

ClassNameCheck pattern: !ruby/regexp /^[A-Z][a-zA-Z0-9]*$/

CyclomaticComplexityBlockCheck 4

CyclomaticComplexityMethodCheck 8

EmptyRescueBodyCheck

ForLoopCheck

MethodLineCountCheck 20

MethodNameCheck pattern: !ruby/regexp /^[_a-z<>=[]|+-/*`]+[_a-z0-9_<>=~@[]]*[=!?]?$/

ModuleLineCountCheck 300

ModuleNameCheck pattern: !ruby/regexp /^[A-Z][a-zA-Z0-9]*$/

ParameterNumberCheck 5


lib/formtastic.rb:8 - Module "Formtastic" has 1272 lines. It should have 300 or less.
lib/formtastic.rb:10 - Class "SemanticFormBuilder" has 1196 lines. It should have 300 or less.
lib/formtastic.rb:742 - Method "date_or_datetime_input" has 33 lines. It should have 20 or less.
lib/formtastic.rb:835 - Method "check_boxes_input" has 24 lines. It should have 20 or less.
lib/formtastic.rb:1008 - Method "default_input_type" has 24 lines. It should have 20 or less.
lib/formtastic.rb:1046 - Method "find_collection_for_column" has 27 lines. It should have 20 or less.
lib/formtastic.rb:1180 - Method "localized_attribute_string" has 24 lines. It should have 20 or less.
lib/formtastic.rb:742 - Method name "date_or_datetime_input" cyclomatic complexity is 10. It should be 8 or less.
lib/formtastic.rb:1008 - Method name "default_input_type" cyclomatic complexity is 19. It should be 8 or less.
lib/formtastic.rb:1046 - Method name "find_collection_for_column" cyclomatic complexity is 10. It should be 8 or less.
lib/formtastic.rb:756 - Block cyclomatic complexity is 5. It should be 4 or less.
lib/formtastic.rb:1027 - Found = in conditional. It should probably be an ==
lib/formtastic.rb:1188 - Rescue block should not be empty.
lib/formtastic.rb:103 - Don't use class variables. You might want to try a different design.


spec/formtastic_spec.rb:38 - Block cyclomatic complexity is 5. It should be 4 or less.
spec/formtastic_spec.rb:109 - Case statement is missing an else clause.


Let’s work through the spec ﬁrst


spec/formtastic_spec.rb:38 - Block cyclomatic
complexity is 5. It should be 4 or less.


spec/formtastic_spec.rb:109 - Case statement
is missing an else clause.


Existing code

Post.stub!(:reflect_on_association).and_return do |column_name|
case column_name
when :author, :author_status
mock('reflection', :options => {},
:klass => Author,
:macro => :belongs_to)
when :authors
:klass => Author,
:macro => :has_and_belongs_to_many)
end
end


New code

def mock_post_association(column_name)
return mock_association(Author, :belongs_to) if column_name.is_post_belongs_to?
return mock_association(Author, :has_and_belongs_to_many) if
column_name.is_post_habtm?
end

def mock_association(klass, multiplicity)
:klass => klass,
:macro => multiplicity)
end
...
Post.stub!(:reflect_on_association).and_return
{ |column_name|mock_post_association(column_name) }


Not worth addressing these


Back to the non-test code


lib/formtastic.rb:742 - Method "date_or_datetime_input" has 33 lines. It should have 20 or less.
lib/formtastic.rb:835 - Method "check_boxes_input" has 24 lines. It should have 20 or less.
lib/formtastic.rb:1008 - Method "default_input_type" has 24 lines. It should have 20 or less.
lib/formtastic.rb:1046 - Method "find_collection_for_column" has 27 lines. It should have 20 or less.
lib/formtastic.rb:1180 - Method "localized_attribute_string" has 24 lines. It should have 20 or less.
lib/formtastic.rb:742 - Method name "date_or_datetime_input" cyclomatic complexity is 10. It should be 8 or less.
lib/formtastic.rb:1008 - Method name "default_input_type" cyclomatic complexity is 19. It should be 8 or less.
lib/formtastic.rb:1046 - Method name "find_collection_for_column" cyclomatic complexity is 10. It should be 8 or less.
lib/formtastic.rb:756 - Block cyclomatic complexity is 5. It should be 4 or less.


(and now I make lots of code changes)


and I decide that’s enough for now


How does this change ﬂog?


Old and new flog warnings

>88.1: SemanticFormBuilder#date_or_datetime_input 60.4: SemanticFormBuilder#inputs
60.4: SemanticFormBuilder#inputs 58.4: SemanticFormBuilder#input
58.4: SemanticFormBuilder#input 45.3: SemanticFormBuilder#database_column_input_type
>57.7: SemanticFormBuilder#find_collection_for_column 43.3: SemanticFormBuilder#field_set_and_list_wrapping
-56.9: SemanticFormBuilder#default_input_type 40.2: SemanticFormBuilder#radio_input
>48.3: SemanticFormBuilder#localized_attribute_string 38.4: SemanticFormBuilder#localized_attribute_string
-44.3: SemanticFormBuilder#check_boxes_input 31.4:
43.3: SemanticFormBuilder#field_set_and_list_wrapping SemanticFormBuilder#find_collection_for_column_explicit_or_asso
40.2: SemanticFormBuilder#radio_input ciation_or_boolean
31.2: SemanticFormBuilder#inputs_for_nested_attributes 31.2: SemanticFormBuilder#inputs_for_nested_attributes
24.0: SemanticFormBuilder#label 29.5: SemanticFormBuilder#datetime_content_tag
22.7: SemanticFormBuilder#method_required? 28.1: SemanticFormBuilder#find_collection_for_column
20.0: SemanticFormBuilder#none 26.0: SemanticFormBuilder#single_list_item_content_from
24.0: SemanticFormBuilder#label
22.7: SemanticFormBuilder#method_required?
20.2: SemanticFormBuilder#date_or_datetime_input
20.0: SemanticFormBuilder#none


Changes

>88.1: SemanticFormBuilder#date_or_datetime_input 45.3: SemanticFormBuilder#database_column_input_type
>57.7: SemanticFormBuilder#find_collection_for_column 38.4: SemanticFormBuilder#localized_attribute_string
-56.9: SemanticFormBuilder#default_input_type 31.4:
>48.3: SemanticFormBuilder#localized_attribute_string SemanticFormBuilder#find_collection_for_column_explicit_
-44.3: SemanticFormBuilder#check_boxes_input or_association_or_boolean
29.5: SemanticFormBuilder#datetime_content_tag
28.1: SemanticFormBuilder#find_collection_for_column
26.0: SemanticFormBuilder#single_list_item_content_from
20.2: SemanticFormBuilder#date_or_datetime_input


So what’s Flog?


Confessions of a Ruby Sadist sudo gem install flog 14/08/09 3:34 AM

Ruby Sadist Flog Flay Heckle Kittens About Us

Confessions of a Ruby Sadist
sudo gem install flog

Flog shows you the most
torturous code you wrote.
The more painful the code,
the higher the score. The
higher the score, the
harder it is to test.

Run it against your best
stuff. I double-dog dare you.
So what’s Flog?
Flog essentially scores an
ABC metric: Assignments,
Branches, Calls, with
particular attention placed
on calls.

Run flog on all your code.
Try this:

find lib -name *.rb | xargs flog

Whatever is at the top of the
report is worth looking at.

This is how it works:


Confessions of a Ruby Sadist sudo gem install flog 14/08/09 3:34 AM

class Test
class Test
def blah # 11.2 =
def blah
a = eval "1+1" # 1.2 + 6.0 +
a = eval "1+1"
if a == 2 then # 1.2 + 1.2 +
if a == 2 then Is seen by
0.4 +
puts "yay" flog as: puts "yay" # 1.2
end
end
end
end
end
end

Test#blah: (11.2)
6.0: eval
1.2: branch
and reported 1.2: ==
as: 1.2: puts
1.2: assignment
0.4: lit_fixnum


Flog weights
THRESHOLD = 0.60
SCORES = Hash.new 1 # various "magic" usually used for "clever code"
BRANCHING = SCORES.merge!(:alias_method => 2,
[ :and, :case, :else, :if, :or, :rescue, :until, :extend => 2,
:when, :while ] :include => 2,
:instance_method => 2,
# various non-call constructs :instance_methods => 2,
OTHER_SCORES = { :method_added => 2,
:alias => 2, :method_defined? => 2,
:assignment => 1, :method_removed => 2,
:block => 1, :method_undefined => 2,
:block_pass => 1, :private_class_method => 2,
:branch => 1, :private_instance_methods => 2,
:lit_fixnum => 0.25, :private_method_defined? => 2,
:sclass => 5, :protected_instance_methods => 2,
:super => 1, :protected_method_defined? => 2,
:to_proc_icky! => 10, :public_class_method => 2,
:to_proc_normal => 5, :public_instance_methods => 2,
:yield => 1, :public_method_defined? => 2,
} :remove_method => 2,
:send => 3,
# eval forms :undef_method => 2)
SCORES.merge!(:define_method => 5,
:eval => 5, # calls I don't like and usually see being
:module_eval => 5, abused
:class_eval => 5,
:instance_eval => 5) SCORES.merge!(:inject => 2)


ﬂog -d lib/formtastic.rb


Flog details for worst method
60.4: SemanticFormBuilder#inputs
16.6: assignment
11.9: branch
11.5: to_proc_normal
5.8: block_pass
5.7: map
4.8: class
3.3: field_set_and_list_wrapping
2.3: content_columns
2.2: macro
2.1: reflections
2.0: ==
2.0: to_sym
1.9: compact!
1.9: +
1.9: -
1.8: empty?
1.8: input
1.5: block_given?
1.5: inputs_for_nested_attributes
1.4: extract_options!
1.4: []


Original code

def inputs(*args, &block)
html_options = args.extract_options!
html_options[:class] ||= "inputs"

if html_options[:for]
inputs_for_nested_attributes(args, html_options, &block)
elsif block_given?
field_set_and_list_wrapping(html_options, &block)
else
if @object && args.empty?
args = @object.class.reflections.map { |n,_| n if _.macro == :belongs_to }
args += @object.class.content_columns.map(&:name)
args -= %w[created_at updated_at created_on updated_on lock_version]
args.compact!
end
contents = args.map { |method| input(method.to_sym) }

field_set_and_list_wrapping(html_options, contents)
end
end


First change

elsif block_given?
else
args = build_default_inputs_args
end
contents = args.map { |method| input(method.to_sym) }
end
end

def build_default_inputs_args()
args.compact!
end


Flog details
29.8: SemanticFormBuilder#build_default_inputs_args
9.0: to_proc_normal
8.7: assignment
3.8: class
3.0: map
2.9: branch
1.8: block_pass
1.7: macro
1.6: reflections
1.5: ==
1.4: compact!
1.4: -
1.4: +

24.1: SemanticFormBuilder#inputs
8.0: branch
6.7: assignment
3.5: block_pass
3.3: field_set_and_list_wrapping
2.0: to_sym
1.8: empty?
1.8: build_default_inputs_args
1.8: input
1.7: map
1.5: inputs_for_nested_attributes
1.5: block_given?
1.4: extract_options!
1.4: []


Next change

elsif block_given?
else
contents = inputs_contents(args)
end
end

def inputs_contents(args)
args = build_default_inputs_args
end
return args.map { |method| input(method.to_sym) }
end

args.compact!
end


Flog details - only one signiﬁcant method

9.0: to_proc_normal
8.7: assignment
3.8: class
3.0: map
2.9: branch
1.8: block_pass
1.7: macro
1.6: reflections
1.5: ==
1.4: compact!
1.4: -
1.4: +


Inline to_proc

args += @object.class.content_columns.map {|c| c.name}
args.compact!
end


Flog details

10.4: assignment
4.5: branch
3.8: class
3.0: map
1.7: name
1.7: macro
1.6: reflections
1.5: ==
1.4: compact!
1.4: -


Extract method

args += content_column_names
args.compact!
end

def content_column_names
return @object.class.content_columns.map {|c| c.name}
end


Flog details

8.7: assignment
2.9: branch
1.8: class
1.7: macro
1.6: content_column_names
1.6: reflections
1.5: ==
1.4: compact!
1.4: map
1.4: -
1.4: +


and so forth...


Other Ruby code quality tools


Code churn metric


Saikuro : A Cyclomatic Complexity Analyzer 14/08/09 5:05 AM

Saikuro
A Cyclomatic Complexity Analyzer
Download Saikuro

Saikuro is a Ruby cyclomatic complexity analyzer. When given
Ruby source code Saikuro will generate a report listing the
cyclomatic complexity of each method found. In addition,
Saikuro counts the number of lines per method and can
generate a listing of the number of tokens on each line of code.

Version:
0.2

License
Saikuro uses the BSD license.

Example Output
Here are examples of the output generated by Saikuro.
Cyclomatic complexity summary page
Token count summary page

Installation
As root run:
# ruby setup.rb all

Usage
Saikuro is a command line program. Running "saikuro.rb -h" will
output a usage statement describing all the various arguments
you can pass to it.
"saikuro -c -p tests/samples.rb"
The above command is a simple example that generates a
cyclomatic complexity report on the samples.rb file, using the
default filter, warning and error settings. The report is saved in
the current directory.
A more detailed example is :
Friday, 28 August 2009 "saikuro -c -t -i tests -y 0 -w 11 -e 16 -o out/" 93

reek, a code smells detector for ruby « silk and spinach 14/08/09 5:06 AM

silk and spinach
development, by example

reek, a code smells detector for ruby
with 23 comments

To complement the imminent (ie. sometime next year) publication of the Ruby Refactoring Workbook
I’ve been working on a little software tool called ‘reek’. Reek scans ruby code — either source files or
in-memory Class objects — looking for some of the code smells discussed in the book. Right now it can
detect (certain forms of):

Long Method
Large Class
Feature Envy
Uncommunicative Name
Long Parameter List
Utility Function
Nested Iterators

As time goes by I’ll be adding checks for more smells, and I hope to make the current checks a bit more
sophisticated. But in the agile spirit of going ugly early, the current very early version is available for
you to experiment with. To get the tool:
gem install reek

To run it:
reek file1.rb file2.rb ...

If you try it, please let me know what you think and what you’d like the next version to do!

You can browse the reek project’s code repository at http://github.com/kevinrutherford/reek/tree/master
or http://rubyforge.org/projects/reek/, both of which are clones of the same Git repository.

RCov - Ruby C0 Code Coverage 15/08/09 12:11 PM

RCov
RCov - Ruby C0 Code Coverage

Source

Install
$ gem install relevance-rcov

Usage
See the README on GitHub for the most-up to date usage of RCov.

Maintained by Relevance


metric fu


rake metrics:all


results at
ﬁle:///Users/stevehayes/projects/third-party/
formtastic/tmp/metric_fu/output/index.html


Java time


Example code is from Apache
Commons Collections 3.2.1


Simian for duplication detection


java -jar simian-2.2.24.jar src/**/*.java


Found 11527 duplicate lines in 871 blocks in 239 files

Processed a total of 41324 significant (109875 raw)
lines in 467 files

Processing time: 2.404sec


27.9% of the code is duplicated


Simian can also be language
sensitive


java -jar simian-2.2.24.jar
-language=java src/**/*.java


PMD for duplication detection


Android Rules: These rules deal with the Android SDK, mostly related to best practices. To get better results, make sure
that the auxclasspath is defined for type resolution to work.
• Basic JSF rules: Rules concerning basic JSF guidelines.
• Basic JSP rules: Rules concerning basic JSP guidelines.
• Basic Rules: The Basic Ruleset contains a collection of good practices which everyone should follow.
• Braces Rules: The Braces Ruleset contains a collection of braces rules.
• Clone Implementation Rules: The Clone Implementation ruleset contains a collection of rules that find questionable usages
of the clone() method.
• Code Size Rules: The Code Size Ruleset contains a collection of rules that find code size related problems.
• Controversial Rules: The Controversial Ruleset contains rules that, for whatever reason, are considered controversial. They
are separated out here to allow people to include as they see fit via custom rulesets. This ruleset was initially created in
response to discussions over UnnecessaryConstructorRule which Tom likes but most people really dislike :-)
• Coupling Rules: These are rules which find instances of high or inappropriate coupling between objects and packages.
• Design Rules: The Design Ruleset contains a collection of rules that find questionable designs.
• Finalizer Rules: These rules deal with different problems that can occur with finalizers.
• Import Statement Rules: These rules deal with different problems that can occur with a class' import statements.
• J2EE Rules: These are rules for J2EE
• JavaBean Rules: The JavaBeans Ruleset catches instances of bean rules not being followed.
• JUnit Rules: These rules deal with different problems that can occur with JUnit tests.
• Jakarta Commons Logging Rules: The Jakarta Commons Logging ruleset contains a collection of rules that find questionable
usages of that framework.
• Java Logging Rules: The Java Logging ruleset contains a collection of rules that find questionable usages of the logger.
• Migration Rules: Contains rules about migrating from one JDK version to another. Don't use these rules directly, rather, use
a wrapper ruleset such as migrating_to_13.xml.
• Migration13: Contains rules for migrating to JDK 1.3
• MigratingToJava4: Contains rules for migrating to JDK 1.5
• Naming Rules: The Naming Ruleset contains a collection of rules about names - too long, too short, and so forth.
• Optimization Rules: These rules deal with different optimizations that generally apply to performance best practices.
• Strict Exception Rules: These rules provide some strict guidelines about throwing and catching exceptions.
• String and StringBuffer Rules: These rules deal with different problems that can occur with manipulation of the class String
or StringBuffer.
• Security Code Guidelines: These rules check the security guidelines from Sun, published at http://java.sun.com/security/
seccodeguide.html#gcg
• Type Resolution Rules: These are rules which resolve java Class files for comparisson, as opposed to a String


Basic Rules: The Basic Ruleset contains a collection of good practices which everyone should follow.
• Braces Rules: The Braces Ruleset contains a collection of braces rules.
• Clone Implementation Rules: The Clone Implementation ruleset contains a collection of rules that find questionable usages
of the clone() method.
• Code Size Rules: The Code Size Ruleset contains a collection of rules that find code size related problems.
• Coupling Rules: These are rules which find instances of high or inappropriate coupling between objects and packages.
• Design Rules: The Design Ruleset contains a collection of rules that find questionable designs.
• Import Statement Rules: These rules deal with different problems that can occur with a class' import statements.
• JUnit Rules: These rules deal with different problems that can occur with JUnit tests.
• Naming Rules: The Naming Ruleset contains a collection of rules about names - too long, too short, and so forth.
• Optimization Rules: These rules deal with different optimizations that generally apply to performance best practices.
• Strict Exception Rules: These rules provide some strict guidelines about throwing and catching exceptions.
• String and StringBuffer Rules: These rules deal with different problems that can occur with manipulation of the class String
or StringBuffer.
• Type Resolution Rules: These are rules which resolve java Class files for comparisson, as opposed to a String
• Unused Code Rules: The Unused Code Ruleset contains a collection of rules that find unused code.


Basic Rules
• Empty catch block • Unnecessary final modifier
• Empty if Statement • Collapsible if statements
• Empty while statement • Useless overriding method
• Empty try block • Class cast exception with toArray
• Empty finally block • Avoid decimal literals in BigDecimal
• Empty switch statements constructor
• Jumbled incrementer • Useless operation on immutable
• For loop should be while loop • Misplaced null check
• Unnecessary conversion temporary • Unused null check in equals
• Override both equals and hashcode • Avoid thread group
• Double checked locking • Broken null check
• Return from finally block • BigInteger instantiation
• Empty synchronized block • Avoid using octal values
• Unnecessary return • Avoid using hardcoded IP
• Empty static initializer • Check result set
• Unconditional if statement • Avoid multiple unary operators
• Empty statement not in loop • Empty initializer
• Boolean instantiation


Basic Rules - 725
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/BeanMap.java:308
Overriding method merely calls super
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ExtendedProperties.java:410
Avoid empty catch blocks
These nested if statements could be combined
Avoid instantiating Boolean objects; reference
Boolean.TRUE or Boolean.FALSE or call Boolean.valueOf() instead.
Avoid instantiating Boolean objects; reference
Boolean.TRUE or Boolean.FALSE or call Boolean.valueOf() instead.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/IteratorUtils.java:907
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/LRUMap.java:120
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/MapUtils.java:170
Avoid instantiating Boolean objects; reference Boolean.TRUE or
Boolean.FALSE or call Boolean.valueOf() instead.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/MultiHashMap.java:138
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ReferenceMap.java:598
Avoid empty while statements
Ensure you override both equals() and hashCode()
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/StaticBucketMap.java:159
Unnecessary final modifier in final class
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/functors/PrototypeFactory.java:195
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/functors/PrototypeFactory.java:202
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/FilterIterator.java:108
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/FilterListIterator.java:140
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/FilterListIterator.java:156
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/ObjectGraphIterator.java:141
Avoid empty if statements
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/list/CursorableLinkedList.java:428
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/AbstractReferenceMap.java:278
Avoid empty while statements
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/CaseInsensitiveMap.java:133
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/HashedMap.java:96
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/IdentityMap.java:175
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/LRUMap.java:395
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/LinkedMap.java:117
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/StaticBucketMap.java:160


Braces Rules

• If statements must use braces
• While loops must use braces
• If else statements must use braces
• For loops must use braces


Braces Rules - 210
Avoid using if statements without curly braces
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/BinaryHeap.java:476
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/BoundedFifoBuffer.java:272
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/FastArrayList.java:370
Avoid using if...else statements without curly braces
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/FastHashMap.java:592
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/FastTreeMap.java:700
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/LRUMap.java:92
Avoid using while statements without curly braces
Avoid using while statements without curly braces


Clone Rules

• Proper clone implementation (should include super.clone())
• Clone throws CloneNotSupportedException
• Clone method must implement cloneable


Clone Rules - 25
Object clone() should be implemented with super.clone()
clone() method should be implemented only if implementing
Cloneable interface
clone() method should throw CloneNotSupportedException
Cloneable interface
Cloneable interface
Cloneable interface
clone() method should throw
CloneNotSupportedException
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/AbstractHashedMap.java:1227
clone() method should be implemented only if
implementing Cloneable interface
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/CaseInsensitiveMap.java:133
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/Flat3Map.java:1014
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/HashedMap.java:96
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/IdentityMap.java:175
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/LRUMap.java:395
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/LinkedMap.java:117
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/MultiKeyMap.java:818
Cloneable interface
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/SingletonMap.java:538
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/BulkTest.java:177


Code Size Rules

• NPath complexity (200)
• Excessive method length (100)
• Excessive parameter list (10)
• Excessive class length (1000)
• Cyclomatic complexity (10)
• Excessive public count (45)
• Too many ﬁelds (15)
• NCSS method count (100)
• NCSS type count (1500)
• NCSS constructor count (100)
• Too many methods (10)


Code Size Rules - 338
This class has too many methods, consider refactoring it.
The class 'BinaryHeap' has a Cyclomatic Complexity of 2 (Highest =
14).
The method 'iterator' has a Cyclomatic Complexity of 14.
The method 'remove' has a Cyclomatic Complexity of 10.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ClosureUtils.java:57
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/CollectionUtils.java:17
his class has a bunch of public methods and attributes
T
void really long classes.
A
he class 'CollectionUtils' has a Cyclomatic Complexity of 3
T
(Highest = 14).
his class has too many methods, consider refactoring it.
T
The method 'index' has a Cyclomatic Complexity of 13.
The method 'get' has a Cyclomatic Complexity of 14.
The method 'size' has a Cyclomatic Complexity of 10.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ComparatorUtils.java:44
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/CursorableLinkedList.java:17
This class has a bunch of public methods and attributes
Avoid really long classes.
T
he method insertListable() has an NPath complexity of
T
200
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/DefaultMapBag.java:48
T
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/DoubleOrderedMap.java:17
This class has a bunch of public methods and attributes
Avoid really long classes.
The class 'DoubleOrderedMap' has a Cyclomatic
Complexity of 4 (Highest = 15).
The method 'entrySetByValue' has a Cyclomatic Complexity
of 12.
The method 'valuesByValue' has a Cyclomatic Complexity
of 11.
The method 'doRedBlackInsert' has a Cyclomatic
Complexity of 11.
The method 'doRedBlackDelete' has a Cyclomatic
Complexity of 13.
The method 'doRedBlackDeleteFixup' has a Cyclomatic
Complexity of 12.
The method 'swapPosition' has a Cyclomatic Complexity of


Coupling Rules

• Coupling between objects (attributes, local variables,
return types) (20)
• Excessive imports (30)
• Loose coupling (avoid using implementation types, use
interface instead)


Coupling Rules - 63
Avoid using implementation types like 'HashMap'; use the interface
instead
instead
instead
instead
Avoid using implementation types like 'ArrayList'; use the
interface instead
Avoid using implementation types like 'Vector'; use the
interface instead
interface instead
interface instead
Avoid using implementation types like 'ArrayList'; use the interface
instead
instead
Avoid using implementation types like 'TreeMap'; use the interface
instead
A high number of imports can indicate a high degree of coupling
within an object.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/SequencedHashMap.java:154
Avoid using implementation types like 'HashMap'; use the
interface instead
interface instead
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/CollatingIterator.java:49
Avoid using implementation types like 'ArrayList';
use the interface instead
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/MultiValueMap.java:78
interface instead
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/MultiValueMap.java:109
interface instead
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/map/StaticBucketMap.java:499
interface instead


Design Rules
• Use singleton (class has nothing but static methods) • Missing static method in non-instantiable class
• Simplify boolean returns • Avoid synchronized at method level
• Simplify boolean expressions • Missing break in switch
• Switch statements should have a default • Use notifyAll() instead of notify()
• Avoid deeply nested if statements • Avoid instanceOf checks in catch clause
• Avoid reassigning parameters • Abstract class without abstract method
• Switch density (statements to labels) • Simplify conditional
• Constructor calls overridable method • Compare objects with equals
• Accessor class generation • Position literals first in comparisons
• Final field should be static • Unnecessary local before return
• Close resource • Non threadsafe singleton
• Non static initializer • Uncommented empty method
• Default label not last in switch statement • Uncommented empty constructor
• Non case label in switch statement • Avoid constants interface
• Optimizable toArray call • Unsynchronized static date formatter
• Bad comparison • Preserve stack trace
• Equals null • Use collection isEmpty()
• Confusing ternary • Class with only private constructors should be final
• Instantiation to get class • Empty method in abstract class should be abstract
• Idempotent operations • Singular field
• Simple date format needs locale • Return empty array rather than null
• Immutable field • Abstract class without any method
• Use locale with case conversions • Too few branches for a switch statement
• Avoid protected field in final class
• Assignment to non final static


Design Rules - 903
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/BagUtils.java:44
All methods are static. Consider using Singleton instead.
Alternatively, you could add a private constructor or make the class abstract to silence this warning.
Document empty constructor
Private field 'readMethods' could be made final; it is only initialized in
the declaration or constructor.
Private field 'writeMethods' could be made final; it is only initialized
in the declaration or constructor.
Private field 'types' could be made final; it is only initialized in the
declaration or constructor.
Document empty constructor
Overridable method 'getBean' called during object construction
New exception is thrown in catch block, original stack trace may be
lost
lost
lost
Deeply nested if..then statements are hard to read
Document empty method
Private field 'owner' could be made final; it is only initialized in the
declaration or constructor.
Avoid reassigning parameters such as 'value'
Consider simply returning the value vs storing it in local variable
'answer'
lost
lost
Avoid if (x != y) ..; else ..;
lost
lost
Avoid reassigning parameters such as 'index'
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/BufferUtils.java:36
All methods are static. Consider using Singleton instead.


Imports Rules

• Duplicate imports
• Don’t import java.lang
• Unused imports
• Import from same package
• Too many static imports


Imports Rules - 0

No problems found!


JUnit Rules

• JUnit static suite
• JUnit spelling
• JUnit assertions should include messages
• JUnit tests should include assert
• TestClass without test cases
• Unnecessary boolean assertion
• Use assertEquals instead of assertTrue
• Use assertSame instead of assertTrue
• Use assertNull instead of assertTrue
• Simplify boolean assertion


JUnit Rules - 1320
This class name ends with 'Test' but contains no test cases
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestAll.java:31
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestAllPackages.java:30
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestClosureUtils.java:97
JUnit assertions should include a message


Naming Rules
• Short variable (3)
• Long variable (17)
• Short method name (3)
• Variable naming conventions
• Method naming conventions
• Class naming conventions
• Abstract naming
• Avoid dollar signs
• Method with same name as enclosing class
• Suspicious hashCode() method name
• Suspicious constant field name
• Suspicious equals method name
• Avoid field name matching type name
• Avoid field name matching method name
• No package
• Package case
• Misleading variable name
• Boolean get method name


Naming Rules - 1433
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ArrayStack.java:88
Avoid variables with short names like n
Avoid variables with short names like m
Avoid variables with short names like i
Avoid excessively long variable names like defaultTransformers
Avoid excessively long variable names like propertyDescriptors
Avoid excessively long variable names like propertyDescriptor
Avoid variables with short names like ex
Avoid variables with short names like ex
Variables that are not final should not contain underscores (except
for underscores in standard prefix/suffix).
Avoid variables with short names like a
Avoid variables with short names like b
Avoid variables with short names like sb
Variables that are not final should not contain underscores
(except for underscores in standard prefix/suffix).
Avoid excessively long variable names like predicatesAndClosures
Avoid excessively long variable names like objectsAndClosures
he field name indicates a constant but its modifiers do not
T
ariables should start with a lowercase character
V
ariables that are not final should not contain underscores (except
V
void variables with short names like a
A
void variables with short names like b
A
void variables with short names like it
A
Avoid variables with short names like a


Optimizations Rules

• Local variable could be ﬁnal
• Method argument could be ﬁnal
• Avoid instantiating objects in loops
• Use ArrayList instead of vector
• Simplify startsWith()
• Use StringBuffer for string appends
• Use arrays asList()
• Avoid array loops
• Unnecessary wrapper object creation
• Add empty string


Optimizations Rules - 7192
Parameter 'initialSize' is not assigned and could be declared final
Local variable 'n' could be declared final
Parameter 'n' is not assigned and could be declared final
Local variable 'm' could be declared final
Local variable 'n' could be declared final
Parameter 'item' is not assigned and could be declared final
Parameter 'object' is not assigned and could be declared final
Local variable 'current' could be declared final
Local variable 'size' could be declared final
Local variable 'size' could be declared final
Parameter 'bag' is not assigned and could be declared final
Parameter 'predicate' is not assigned and could be declared final
Parameter 'type' is not assigned and could be declared final
Parameter 'transformer' is not assigned and could be declared final
Parameter 'predicate' is not assigned and could be declared final
Parameter 'type' is not assigned and could be declared final
Parameter 'transformer' is not assigned and could be declared final
Parameter 'input' is not assigned and could be declared final
Parameter 'bean' is not assigned and could be declared final
Local variable 'newMap' could be declared final
Local variable 'readableKeys' could be declared final
Local variable 'key' could be declared final
Parameter 'map' is not assigned and could be declared final
Local variable 'readableKeys' could be declared final


Strict Exceptions Rules

• Avoid catching Throwable
• Signature declares throws Exception
• Exceptions as ﬂow control
• Avoid catching NullPointerException
• Avoid throwing raw exception types
• Avoid throwing NulllPointerException
• Avoid rethrowing exception
• Do not extend java.lang.Error
• Do not throw exception in ﬁnally
• Avoid throwing new instances of the same exception


Strict Exception Rules - 125
Avoid catching NullPointerException; consider removing the cause of
the NPE.
Avoid throwing null pointer exceptions.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/ComparatorUtils.java:96
Avoid catching NullPointerException; consider removing the
cause of the NPE.
cause of the NPE.
cause of the NPE.
Avoid throwing raw exception types.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/SequencedHashMap.java:777
Avoid throwing raw exception types.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/UnboundedFifoBuffer.java:125
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/bidimap/TreeBidiMap.java:1263
cause of the NPE.
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/buffer/BoundedFifoBuffer.java:233


Strings Rules
• Avoid duplicate literals
• String instantiation
• String toString()
• Inefficient string buffering
• Unnecessary case change
• Use StringBuffer.length()
• Append character with char
• Consecutive literal appends
• Use indexOf(char)
• Inefficient empty string check
• Insufficient StringBuffer declaration
• Useless String.valueOf()
• StringBuffer instantiation with char
• Use equals() to compare strings
• Avoid StringBuffer field


Strings Rules - 214
No need to call String.valueOf to append to a string.
Avoid appending characters as strings in StringBuffer.append.
The String literal "' doesn't map to an existing object"
appears 6 times in this file; the first occurrence is on line 1,143
The String literal "Iterator must not be null" appears 7 times in this
file; the first occurrence is on line 618
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/bidimap/AbstractDualBidiMap.java:450
The String literal "Iterator remove() can only be
called once after next()" appears 4 times in this file; the first occurrence is on line 450
The String literal "Map is empty" appears 4 times in this file;
the first occurrence is on line 291
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/buffer/BlockingBuffer.java:143
The String literal "Caused by InterruptedException: "
appears 4 times in this file; the first occurrence is on line 143
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/functors/FunctorUtils.java:68
The String literal " was null" appears 4 times in this file; the
first occurrence is on line 68
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/AbstractEmptyIterator.java:43
The String literal "Iterator contains no elements"
appears 7 times in this file; the first occurrence is on line 43
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/iterators/LoopingListIterator.java:90
The String literal "There are no elements for this
iterator to loop on" appears 4 times in this file; the first occurrence is on line 90
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/list/AbstractLinkedList.java:364
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/list/AbstractLinkedList.java:376
/Users/stevehayes/commons-collections-3.2.1-src/src/java/org/apache/commons/collections/list/FixedSizeList.java:72
The String literal "List is fixed size" appears 11 times in this
file; the first occurrence is on line 72
StringBuffer.append is called 2 consecutive times
with literal Strings. Use a single append with a single String.
he String literal "Invalid map index" appears 4 times in this file; the
T
first occurrence is on line 629
The String literal "(this Map)" appears 6 times in this file; the
first occurrence is on line 1,115


Unused Code Rules

• Unused private ﬁeld
• Unused local variable
• Unused private variable
• Unused formal parameter


Unused Code Rules - 68
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/AbstractTestObject.java:170
Avoid unused local variables such as 'p'./Users/stevehayes/
commons-collections-3.2.1-src/src/test/org/apache/commons/collections/MapPerformance.java:54
Avoid unused local variables such as 'unmodHashMap'./Users/stevehayes/
commons-collections-3.2.1-src/src/test/org/apache/commons/collections/MapPerformance.java:55
Avoid unused local variables such as 'fastHashMap'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/MapPerformance.java:56
Avoid unused local variables such as 'treeMap'.
Avoid unused local variables such as 'seqMap'.
Avoid unused local variables such as 'total'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestBinaryHeap.java:313
Avoid unused local variables such as 'fail'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestCursorableLinkedList.java:975
Avoid unused local variables such as 'cursor'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestFactoryUtils.java:146
Avoid unused local variables such as 'dest'.
Avoid unused local variables such as 'created'.
Avoid unused local variables such as 'factory'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestIteratorUtils.java:250
Avoid unused local variables such as 'x'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestListUtils.java:154
Avoid unused local variables such as 'list'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestListUtils.java:171
Avoid unused local variables such as 'list'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestReferenceMap.java:203
Avoid unused private methods such as 'gc()'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestTransformerUtils.java:422
Avoid unused local variables such as 'trans'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/TestTransformerUtils.java:426
Avoid unused local variables such as 'trans'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestPredicatedBag.java:115
Avoid unused local variables such as 'bag'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestPredicatedBag.java:121
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestPredicatedSortedBag.java:83
Avoid unused local variables such as 'bag2'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestTypedBag.java:107
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestTypedBag.java:113
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bag/TestTypedSortedBag.java:73
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bidimap/AbstractTestSortedBidiMap.java:391
Avoid unused local variables such as
'fromEntry'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/bidimap/AbstractTestSortedBidiMap.java:580
Avoid unused local variables such as
'fromEntry'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/buffer/TestPredicatedBuffer.java:83
Avoid unused local variables such as 'o'.
/Users/stevehayes/commons-collections-3.2.1-src/src/test/org/apache/commons/collections/iterators/AbstractTestMapIterator.java:138
Avoid unused local variables such as 'map'.


FindBugs

http://ﬁndbugs.sourceforge.net/


Implements 370 checks


Description Category
AM: Creates an empty jar file entry Bad practice
AM: Creates an empty zip file entry Bad practice
BC: Equals method should not assume anything about the type of its argument Bad practice
BC: Random object created and used only once Bad practice
BIT: Check for sign of bitwise operation Bad practice
CN: Class implements Cloneable but does not define or use clone method Bad practice
CN: clone method does not call super.clone() Bad practice
CN: Class defines clone() but doesn't implement Cloneable Bad practice
Co: Abstract class defines covariant compareTo() method Bad practice
Co: Covariant compareTo() method defined Bad practice
DE: Method might drop exception Bad practice
DE: Method might ignore exception Bad practice
DMI: Don't use removeAll to clear a collection Bad practice
DP: Classloaders should only be created inside doPrivileged block Bad practice
DP: Method invoked that should be only be invoked inside a doPrivileged block Bad practice
Dm: Method invokes System.exit(...) Bad practice
Dm: Method invokes dangerous method runFinalizersOnExit Bad practice
ES: Comparison of String parameter using == or != Bad practice
ES: Comparison of String objects using == or != Bad practice
Eq: Abstract class defines covariant equals() method Bad practice
Eq: Equals checks for noncompatible operand Bad practice
Eq: Class defines compareTo(...) and uses Object.equals() Bad practice
Eq: equals method fails for subtypes Bad practice
Eq: Covariant equals() method defined Bad practice
FI: Empty finalizer should be deleted Bad practice
FI: Explicit invocation of finalizer Bad practice
FI: Finalizer nulls fields Bad practice
FI: Finalizer only nulls fields Bad practice
FI: Finalizer does not call superclass finalizer Bad practice
FI: Finalizer nullifies superclass finalizer Bad practice
FI: Finalizer does nothing but call superclass finalizer Bad practice
GC: Unchecked type in generic call Bad practice

Some are the same as checks in PMD,
some are different


Checkstyle

http://checkstyle.sourceforge.net/


Implements 128 checks


AbstractClassName Ensures that the names of abstract classes conforming to some regular expression.
AnnotationUseStyle This check controls the style with the usage of annotations.
AnonInnerLength Checks for long anonymous inner classes.
ArrayTrailingComma Checks if array initialization contains optional trailing comma.
ArrayTypeStyle Checks the style of array type definitions.
AvoidInlineConditionals Detects inline conditionals.
AvoidNestedBlocks Finds nested blocks.
AvoidStarImport Check that finds import statements that use the * notation.
AvoidStaticImport Check that finds static imports.
BooleanExpressionComplexity Restricts nested boolean operators (&&, ||, &, | and ^) to a specified depth (default = 3).
ClassDataAbstractionCoupling This metric measures the number of instantiations of other classes within the given class.
ClassFanOutComplexity The number of other classes a given class relies on.
ClassTypeParameterName Checks that class type parameter names conform to a format specified by the format property.
ConstantName Checks that constant names conform to a format specified by the format property.
CovariantEquals Checks that if a class defines a covariant method equals, then it defines method equals(java.lang.Object).
CyclomaticComplexity Checks cyclomatic complexity against a specified limit.
DeclarationOrder Checks that the parts of a class or interface declaration appear in the order suggested by the Code
Conventions for the Java Programming Language.
DefaultComesLast Check that the default is after all the cases in a switch statement.
DescendantToken Checks for restricted tokens beneath other tokens.
DesignForExtension Checks that classes are designed for inheritance.
DoubleCheckedLocking Detect the double-checked locking idiom, a technique that tries to avoid synchronization overhead but is
incorrect because of subtle artifacts of the java memory model.
EmptyBlock Checks for empty blocks.
EmptyForInitializerPad Checks the padding of an empty for initializer; that is whether a space is required at an empty for
initializer, or such spaces are forbidden.
EmptyForIteratorPad Checks the padding of an empty for iterator; that is whether a space is required at an empty for iterator,
or such spaces are forbidden.
EmptyStatement Detects empty statements (standalone ';').
EqualsAvoidNull Checks that any combination of String literals with optional assignment is on the left side of an equals()
comparison.
EqualsHashCode Checks that classes that override equals() also override hashCode().
ExecutableStatementCount Restricts the number of executable statements to a specified limit (default = 30).
ExplicitInitialization Checks if any class or object member explicitly initialized to default for its type value (null for object
references, zero for numeric types and char and false for boolean.
FallThrough Checks for fall through in switch statements Finds locations where a case contains Java code - but lacks a
break, return, throw or continue statement.
FileLength Checks for long source files.
Friday, 28 August 2009
FileTabCharacter Checks to see if a file contains a tab character. 142

Some are the same as checks in PMD and
Findbugs, some are different


All these tools allow ﬁltering


PMD at the ﬁle level


Findbugs and Checkstyle at the ﬁle + check level


You can add your own checks to any of these, all
are open source


You can use them at the command line, from Ant,
or from Maven


There’s probably a plugin for your IDE as well


Many people use these in combination


Sonar

http://sonar.codehaus.org/


Download Features Screencasts Roadmap Documentation Community Blog

Code quality management platform
Sonar enables you to collect, analyze and report metrics on
source code. Sonar not only offers consolidated reporting on and
across projects throughout time, but it becomes the central place
to manage code quality

What is Sonar? Why use Sonar? Java & more

Get Sonar
Monitoring metrics
Projects at risk are quickly and visually detected through the dashboard Download the latest version,
that combines collected measures and calculated metrics on all your 1.10, on August 14, 2009
projects

Get Support
Time machine Choose what fits best your
needs, Professional or
Time machine highlights trends of the main metrics in real time : agile teams
Community support
can react rapidly. In case of outsourced projects, SONAR enables to set
contractual quality objectives
Plugins
Market standards Extend the functionality with
open source and
Metrics related to coding conventions integrate more than 300 market commercial plugins
best-practice rules. Those rules are grouped into families to offer a
synthetic view (maintainability, reliability …)
Sonar in action
To find out more, watch a
Open source components screencast or visit our public
instance
SONAR leverages the existing ecosystem of quality open source tools (ex. Checkstyle,
PMD, Maven, Cobertura …), to offer a fully integrated solution to development
environments and continuous integration tools Blog
Subscribe to our blog by email or
directly through the feed
Easy navigation Sonar 1.10 in screenshots
SONAR enables to navigate seamlessly through various Source code analysis is not an
granularity levels, from projects portfolio to source code level. This end in itself, but a means to an
gives transparency at all level (project, component, package …) end
and enables the developer to take immediate and targeted actions Sonar TV : configuring coding
rules

Powered by SonarSource
Visit Nemo, the live instance of Sonar


Sonar integrates easily into projects that use
Maven2


It also provides trend graphs once it has been
installed


Java code coverage


Emma (http://emma.sourceforge.net/)

Cobertura (http://cobertura.sourceforge.net/)

Clover (http://www.atlassian.com/software/clover/)


Differ in the mechanics of how code coverage is
implemented


Cobertura and Clover let you fail a build if a
coverage threshold is not met


Clover is commercial-ware with extra
functionality:

- run last-failed tests ﬁrst
- distribute test execution


Macker

http://www.innig.net/macker/index.html


Check for architectural layering by deﬁning access
rules between classes and packages


<?xml version="1.0"?>
<macker>
<ruleset name="Simple example">
<access-rule>
<deny>
<from class="**Print*" />
<to class="java.**" />
</deny>
</access-rule>
</ruleset>
</macker>


public class PrintHelloWorld
{
public static void main(String[] args)
{ System.out.println("Hello world, as they say!"); }
}

Checking ruleset: Simple example ...

Illegal reference
from PrintHelloWorld
to java.io.PrintStream

Illegal reference
to java.lang.Object

Illegal reference
to java.lang.System


<?xml version="1.0"?> <deny>
<to>
<macker> <include class="java.sql.**" />
<ruleset name="Layering rules"> <include class="javax.sql.**" />
<var name="base" <include class="javax.jdo.**" />
value="net.innig.macker.example.layering" /> </to>
<pattern name="gui" class="${base}.gui.**" /> </deny>
<pattern name="model" class="${base}.model.**" /> <allow> <from pattern="persist" /> </allow>
<pattern name="persist" class="$ </access-rule>
{base}.persistence.**" />
<access-rule>
<access-rule> <message>Swing and AWT calls belong in the GUI
<message>Only the model can talk to the layer</message>
persistence layer</message> <deny>
<deny> <to>
<to pattern="persist" /> <include class="java.awt.**" />
<allow> <from pattern="persist"/> </allow> <include class="javax.swing.**" />
<allow> <from pattern="model"/> </allow> </to>
</deny> </deny>
</access-rule> <allow> <from pattern="gui" /> </allow>
</access-rule>
<access-rule>
<message>The persistence layer can't access the </ruleset>
model</message> </macker>
<deny>
<to pattern="model" />
<from pattern="persist" />
</deny>
</access-rule>

<access-rule>
<message>The model and persistence layers can't
access the GUI</message>
<deny>
<to pattern="gui" />
<from>
<include pattern="model" />
<include pattern="persist" />
</from>
</deny>
</access-rule>

<access-rule>
<message>Persistence belongs in the persistence
layer</message>


Programmers are inherently human


They inevitably make mistakes


Automation helps reduce the cost of human
mistakes


Code Quality in Ruby and Java

In this document

More Related Content

What's hot

Similar to Code Quality in Ruby and Java

Recently uploaded

Code Quality in Ruby and Java