Posts Tagged sonar

Through the eyes of sonar : equals & hashCode.

Equals and hashCode

Theory

equals

Indicates whether some other object is “equal to” this one.

The equals method implements an equivalence relation on non-null object references:

— It is reflexive: for any non-null reference value x, x.equals(x) should return true.
— It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
— It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
— It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
— For any non-null reference value x, x.equals(null) should return false.

The equals method for class Object implements the most discriminating possible equivalence relation on objects; that is, for any non-null reference values x and y, this method returns true if and only if x and y refer to the same object (x == y has the value true).

Note that it is generally necessary to override the hashCode method whenever this method is overridden, so as to maintain the general contract for the hashCode method, which states that equal objects must have equal hash codes.

hashCode

Returns a hash code value for the object. This method is supported for the benefit of hashtables such as those provided by java.util.HashMap or java.util.HashSet.

The general contract of hashCode is:

— Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer, provided no information used in equals comparisons on the object is modified. This integer need not remain consistent from one execution of an application to another execution of the same application.
— If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.
— It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables.

As much as is reasonably practical, the hashCode method defined by class Object does return distinct integers for distinct objects. (This is typically implemented by converting the internal address of the object into an integer, but this implementation technique is not required by the JavaTM programming language.)

Recommandations

Use the commons lang classes to help your code to be “nullsafe” and easier to implement :

EqualsBuilder
HashCodeBuilder

If you are using hibernate :
— use the getter to avoid “proxy issues”
— avoid using the id as equality (which isn’t always filled)
— read the faq

If you implement them… unit test your equals/hashCode
— There’s a EqualsHashCodeTestCase that you can extends in the junit-addons library
— note that there are also constraint for serialisation/clone
— don’t call hashCode() but toHashCode() on hashCodeBuilder 😉
— avoid reflection based equals/hashCode.

Unit testing

the EqualsHashCodeTestCase is extended in order to test a class’s functional compliance with the equals and hashCode contract.
Override my createInstance and createNotEqualInstance methods to provide me with objects to test against. Both methods should return objects that are of the same class.
will test :
— whether equals holds up against a new Object (should always be false).
— whether equals holds up against null.
— whether equals holds up against objects that should not compare equal.
— whether equals is consistent.
— whether equals is reflexive.
— whether equals is symmetric and transitive.
— the hashCode contract.
— the consistency of hashCode Across Invocations .

Unit Tests and Serialization and serialization

If you adopt a unit-testing methodology, then any serializable class should pass the following three tests:
— If it implements readObject(), it should implement writeObject(), and vice-versa.
— It is equal (using the equals() method) to a serialized copy of itself.
— It has the same hashcode as a serialized copy of itself.

Similar constraints hold for classes that implement the Externalizable interface.

Sonar rules

Warn – eclipse generated equals check class

	public boolean equals(Object obj) {
		if (this == obj)
			return true;
		if (obj == null)
			return false;
		if (getClass() != obj.getClass()) // BAD IDEA IF USING HIBERNATE
			return false;

Prefer instanceof check, because the “obj” may be a HibernateProxy dynamically extending MyObject

	public boolean equals(Object obj) {
		if (this == obj){
			return true;
                }
		if (obj == null) {
			return false;
                }
		if (!(obj instanceof MyObject)) {
			return false;
		}

For the fields comparisons… prefer the EqualsBuilder

see http://tomaszdziurko.pl/2009/05/eclipse-and-problem-with-auto-generated-equals/

Bad practice – Class defines equals() and uses Object.hashCode()

This rule checks that classes that override equals() also need to override hashCode(). If you implement equals… you should implement hashCode accordingly.

Bad practice – equals() method does not check for null argument

As stated in the theory… “whether equals holds up against null”.
A simple check with instanceof… could do the trick.

 public boolean equals(final Object object) {
   if (object.getClass().equals(this.getClass())) {
 	DecisionDetail decisionDetail = (DecisionDetail) object;
    	if (getId() == null || decisionDetail.getId() == null) {
    		return object == this;	
   	}
   }
 }

See openpojo to test your model against such issue.

Correctness – equals method always returns false / Correctness – equals method always returns true

Having an equals method like this isn’t a good idea and doesn’t follow the contract on these methods

//BAD
 public boolean equals(Object object) {		
   return true; // or false			
 }

If you want a good default implementation

//GOOD
 public boolean equals(Object object) {		
   return this==object;
 }

Performance – hashCode return a constant

Returning a constant for the hashCode isn’t good for performance, as the hash is use to “compartiment/bucketing” data in structure like HashSet/hashMap.
Returning a constant will put all the element in the same bucket transforming a lookup of O(1) to O(n) function.

If you don’t have a clean equals criteria fallback to default implementation :

  @Override
  public int hashCode() {
 	return System.identityHashCode(this);
  }
  public boolean equals(Object object) {		
   return this==object;
  }

Correctness – equals() used to compare array and nonarray

Comparing apples and oranges, an array of X won’t be equals to an object instance X

  !PersonTypeCode.ALL.equals(searchParameter.getPersonTypeCode())

Where

  public class PersonTypeCode extends Code {
      public static final PersonTypeCode[] ALL = { PROSPECT, MEMBER, PROSPECT_OR_MEMBER, PROSPECT_FAMILY_MEMBER };
      public static final PersonTypeCode PROSPECT = new PersonTypeCode("PROSPECT");
      ...
   }

Equals vs == :Literal Equality

Literal Strings should be compared using equals(), not ‘!=’.

 String text = ...
 if (text != "") { //BAD
   ...
 }

Ideally… you should use StringUtils to compare in a null safeway.
Note that you have StringUtils.isEmpty(...) or StringUtils.isBlank(...)

 String text = ...
 if (!StringUtils.isEmmpty(text)) {
    ...
 }

Class defines equal(Object); should it be equals(Object)?

Surely a typo. If it’s not a typo choose a better name.

Class defines hashcode(); should it be hashCode()?

Surely a typo. If it’s not a typo choose a better name.

Class defines tostring(); should it be toString()?

Surely a typo. If it’s not a typo choose a better name.

Dodgy – Potentially dangerous use of non-short-circuit logic

 return equalsRoot(other) & new EqualsBuilder().append(this.sourceId, other.getSourceId()).isEquals();

This code seems to be using non-short-circuit logic (e.g., & or |) rather than short-circuit logic (&& or ||). In addition, it seem possible that, depending on the value of the left hand side, you might not want to evaluate the right hand side (because it would have side effects (hibernate initialization), could cause an exception (NullPointerException) or could be expensive (hibernate initialization).

 String s = null;   
 if (s == null || s.trim().length() == 0) {  
     // String s is really empty. Do some stuff.  
 }      
 if (s != null && s.trim().length() != 0) {  
    // String s is not empty. Do some stuff.  
 } 

Both examples would throw a NullPointerException when s is null and you use | and & respectively. (Prefer StringUtils for this kind of check)

Non-short-circuit logic causes both sides of the expression to be evaluated even when the result can be inferred from knowing the left-hand side. This can be less efficient and can result in errors if the left-hand side guards cases when evaluating the right-hand side can generate an error.

, , ,

Leave a comment

Personal taste for test and quality toolbox

TL;DR

* Software Quality Axes
  - Focus on test
  - Quality of a good test
* Libraries to write maintainable tests
  - Junit, fest, mockito, openpojo, spring-mock
  - Build forge with jenkins and sonar
* IDE features and plugins
  - Eclipse : shortcuts, static imports, eclipse template for junit4, eclipse save actions
  - Plug-ins : more unit, Eclemma codecoverage, infinitest, sonar ide
* Coding style
  - Test as running specifications
  - Write testable code
  - Junit best practices
* Conclusion

Software Quality Axes


Sonar is defining software quality through these 7 axes. I will mainly focus on unit tests and their code coverage, maintainability and make them fun to write.

As Java developers we are really lucky,

  • there’s a plethora of libraries to write easily, maintainable tests and
  • we have also a great IDE with a lot of plug-ins and built-in feature to maximize our productivity !

Quality of a good test from Kent Beck :

  • isolated (unaffected by the presence, absence, or results of other tests)
  • automated
  • easy to write
  • easy to read :  expressive -> maintainable (fix or add new ones)
  • easy to run
  • unique : don’t overlap with others -> code coverage

I picked up some plugins and libraries to help on these areas.

Libraries to write maintainable tests


JUnit (easy to run)

JUnit3’s last release was in 2002. So it’s time to move to junit4 ! All the tooling around junit is available : ide, maven, jenkins, sonar,…
Don’t forget to use the ‘new’ features like

I know testNg do this better… but junit is so widespread in my development environment.

Fest Assert(easy to read)

Default junit assertion are not really expressive, custom errors messages are often required to make failure more understandable to the developer. Fest-Assert is a library that let you as a developer write assertion fluently like assertThat(frodo.getAge()).isEqualTo(33); or assertThat(fellowshipOfTheRing).hasSize(9);. More samples in this snippet. After reading this article you will understand why I prefer fest-assert over hamcrest. Note also that festAssert can be extended to support your model : find a sample for jodatime assertions

(easy to write)

Mockito is a mocking framework that tastes really good. It lets you write beautiful tests with clean & simple API. Mockito doesn’t give you hangover because the tests are very readable and they produce clean verification errors. Mockito will help you testing nominal use cases but also extreme situation like unexpected/rare/hard to reproduce Exception (like db connection issues, corrupted database,…).

org.springframework.mock.web (easy to write)

The org.springframework.mock.web package contains a comprehensive set of Servlet API mock objects, targeted at usage with Spring’s Web MVC framework, which are useful for testing web contexts and controllers. These mock objects are generally more convenient to use than dynamic mock objects such as mockito.

Openpojo (easy to write)

Trivializing POJO testing and Identity.

It’s perhaps no a great idea to test getters/setters. The main point with this library is in fact to let you focus on the non trivial code by covering the trivial one. You will be surprised how easy it is when you use openpojo.You can also easily add your own testers for example verifying equals/hashCode transitity, reflectivity, nullsafety,..

Build forge based on

(easy to run)
Getting fast feedback from your commit or the one from your coworker is what jenkins provides you.

Sonar will follow the evolution of your project taking a look at potential bugs, code duplications, uncovered code, fragile test, integration test,…

 IDE features and plugins


Eclipse shortcuts (easy to write)

To be a productive programmer, don’t use the mouse 😉 Eclipse offers already a lot of shortcuts.My favorites : ctrl-shift-T or ctrl-shift-R for a goto type or resources
If you don’t know them well, some articles here and here and a great plugin to learn them.

Static imports (easy to write)

Open Preferences (Window -> Preferences)
Browse to Java -> Editor -> Content Assist -> Favorites
Click 'New Type...'
enter org.junit.Assert.*
enter org.mockito.Mockito.*
enter org.mockito.Matchers.*
enter org.fest.assertions.Assertions.*

If you type mock and then press Ctrl + Space, the static method Mockito.mock is proposed. The static import is added.
see it in action here

Eclipse template for junit4 (easy to write)

These templates once imported will help you to produces junit4 test method via content assist.

enter test
Ctrl + Space and pick test4
your test method is generated just fill the name of the testcase

Eclipse save actions

Sonar is checking unused import, and the systematic use of braces. The good news is that you can configure eclipse to fix these 2 plagues automatically with save-actions

Under "Preferences": Java > Editor > Save Actions
Check "Additional actions"
Click "Configure…"
Go to the "Code Style" tab
Check "Use blocks in if/while/for/do statements" and configure to your preferences

plugin (easy to write and run)

MoreUnit assist you in writing more unit tests. With feature like easy navigation between code and unit test, skeleton generation,…

ctrl-j jump to test or code
ctrl-r run associated test

If you don’t really follow the same naming convention as proposed by the plugin you can modify the preference ‘enable extended search for test methods:’
If needed there’s a similar plugin : fast code

Eclemma codecoverage plugin (unique and isolated)

Detecting uncovered code allows you to easily identify missing unit tests.
Eclemma brings code coverage analysis directly into the Eclipse workbench:

  • Fast develop/test cycle:Launches from within the workbench like JUnit test runs can directly be analyzed for code coverage.
  • Rich coverage analysis:Coverage results are immediately summarized and highlighted in the Java source code editors.
  • Non-invasive: EclEmma does not require modifying your projects or performing any other setup.

(easy to run)

Infinitest is a never ending unit tests runner, displays assertions failure or errors as problem just like compilation errors ! To make infinitest your new best friend you need a fast test suites or filter out the slow ones.

Let’s see fest and infinitest in action : errors displayed in front of contains and the fest message shows the actual content of the fellowshipOfTheRing

plugin

Setting up checkstyle, findbugs, pmd in all your developers workspaces, keeping all the plugins and rules up to date is quite hard. This last plugin will cover all the other axes. A good habit is to launch it when you think you are done… you will perhaps discover that you are not done yet 😉

The Sonar Eclipse Plugin provides a comprehensive integration of Sonar in Eclipse for Java projects. The objective of this integration is that developers do not leave their favorite IDE anymore to manage the quality of their source code. Most information displayed in the Sonar Web interface is now available in Eclipse. You can run local analysis based on the ruleset configured in your central sonar server.

Coding style


Writing tests is always easier if your code is designed to be testable and your test shows the intent of the test.

writing unit test as running specifications :

A good naming convention make it clear what you expect from the MailInboxSyncer implementation.

Class MailInboxSyncerTest {
     @Test
     itShouldSyncMessagesFromPopSinceLastSync() {...}
     @Test
     itShouldCloseConnectionEvenWhenExceptionThrown() {...}
     @Test
     itShouldSyncMessagesOnlyWhenNotAlreadyInInbox() {...}
     @Test
     itShouldIgnoreMessagesOlderThenLastSync() {...}
     @Test
     itShouldIgnoreMessagesFailingFilter() {...}
     @Test
     itShouldDefaultToDefaultTimeWhenNeverSynced() {...}
}

Write testable code

In general, don’t follow any rules from this article on How To Write Unmaintainable Code and Ensure a job for life ;-).
Global state is undesirable (statics, singleton,…) see testability-explorer and then injectability is good : dataSource=new org.apache.commons.dbcp.BasicDataSource() vs setDataSource(Datasource datasource).

Code Design principles that could help having a code base more testable

Layered design
Code against interfaces
Put the code where it belongs
Prefer composition over inheritance.
Favor polymorphism over conditionals
Avoid static methods and the Singleton pattern.
Isolate dependencies.
Inject dependencies

Code practices that don’t help

Mixing of concerns (business logic, caching, transaction,...)
Mixing service objects with value objects
Kilometric classes/methods
A lot a dependency between your classes
Do complicated creation work in objects
Avoid interface usage, depends on concrete
Static methods/fields
Satefull singletons
A lot of global attributes
Make the scope of variable broader then required

Junit Best practices

donts

Don't write unit test without any assert
Don't log… but assert
Don't write complex condition logic : something you are not testing or something that is too complex that should be re-factored.
Avoid hard-coding dataset
Avoid over complex assert

do

Small and fast test
No side effect
A bug means a new unit test
Externalize your dataset if possible (xml,csv,...)
Avoid time dependency via jodatime

For spring integration test : @RunWith(SpringJUnit4ClassRunner.class) Assert result (not null, empty,…) Delta assertion : The pattern is like this: 1.measure 2.exercise 3.measure second times and 4.assert on deltas Guard Assertion : You assert some state before exercising the SUT. For ex an empty case for current dataset Create custom assertions if needed : When complex assertion are “copy-pasted” over the test code or when the assertion code “hides” the goals of the test you can “extract the code” in assertion method.

Conclusion


No excuse ! You have a now my personal toolbox to write more (maintainable) tests.
I’d like to thank all these developers contributing to a better quality, more productivity, gain in visibility via their tools and libraries !

Anything missing from this list ?
Do you have experience with bdd frameworks ?
Share your own testing pearls ?

, , , , , ,

7 Comments

Dangerous can be dating in java , joda and sonar to the rescue!

Don’t use java built-in classes, use jodatime and enforce this rule with sonar !

Don’t use java built-in classes

How many bugs in 5 lines of code ?

Date date = new Date(2007, 12, 13, 16, 40);
TimeZone zone = TimeZone.getInstance("Europe/Bruxelles");
Calendar cal = new GregorianCalendar(date, zone);
DateFormat fm = new SimpleDateFormat("HH:mm Z");
String str = fm.format(cal);

Just 6 bugs !

int year = 2007 - 1900;
int month = 12 - 1;
Date date = new Date(year, month, 13, 16, 40);
TimeZone zone = TimeZone.getInstance("Europe/Brussels");
Calendar cal = new GregorianCalendar(zone);
cal.setTime(date);
DateFormat fm = new SimpleDateFormat("HH:mm Z");
fm.setTimeZone(zone);
Date calDate = cal.getTime();
String str = fm.format(calDate);

If you want deeper explanations see this [presentation]

java.util.Date issues

  • From JDK1.0
  • Uses two digit years (from 1900)
  • January is 0, December is 11
  • Should have been immutable
  • Most methods deprecated in JDK1.1
  • Uses milliseconds from 1970 representation

java.util.Calendar issues

  • From JDK1.1
  • Uses subclasses for different calendar systems
  • January is 0, December is 11
  • Should have been immutable
  • Uses dual representation internally
    • value for each field
    • milliseconds from 1970 representation
  • Odd performance and bugs

java.util.DateFormat issues

  • Pattern based date formatting
  • “dd MMM yyyy”
  • Requires Date object
  • Not thread-safe : see rule findbugs : Multithreaded correctness – Call to static DateFormat
  • Not especially fast
  • Sun RFE to make thread-safe ignored

SQL – java.util.sql.Date, Time, Timestamp issues

Subclass java.util.Date

  • Date extends Date (!)
  • Time extends Date (!)
  • Override superclass to block methods (throws Exception)
  • Timestamp adds nanoseconds
  • equals() broken
  • All the problems of java.util.Date and more
  • timezone problem new Time(long)

Avoid millis manipulation and let’s use Jodatime !

when playing with java.util.Date you end up doing calculation in millis

int days = 40;
Date now = new Date();
long nowMillis = now.getTime();
Timestamp nowTimestamp = new Timestamp(nowMillis);
long future = 3600 * 24 * days * 1000;
Timestamp expiryTimestamp = new Timestamp(nowMillis + future);
System.out.println("nowTimestamp " + nowTimestamp);
System.out.println("expiryTimestamp " + expiryTimestamp);

this last code sample contains a bug… int vs long for days !

see this explaination the expiryTimestamp is before the nowTimestamp for “large days count”

nowTimestamp 	2011-02-04 12:45:40.381
expiryTimestamp 2011-01-25 19:42:53.085

now let’s write the same code with joda time

DateTime nowTimestamp2 = new DateTime();
System.out.println("nowTimestamp    " + nowTimestamp2);
System.out.println("expiryTimestamp " + nowTimestamp2.plusDays(days));

it’s more readable… and most important it return the correct value 😉

nowTimestamp 	2011-02-04T12:45:40.443+01:00
expiryTimestamp 2011-03-16T12:45:40.443+01:00

Sonar to the rescue

sonar can detect these issues :

  • Multithreaded correctness – Call to static Calendar
  • Multithreaded correctness – Call to static DateFormat

to fix them with jodatime let’s use the DateTimeFormat.

fmt=DateTimeFormat.forPattern("MMMM, yyyy");
DateTime datetime = fmt.parseDateTime(duedate);

this one is threadsafe and can be static and final field. or use the toString

datetime.toString("dd:MM:yy");

but the step further is to banish jdk date from your code base !
To do so, let’s define sonar architectural constraint like

  • Arch : avoid java.util.GregorianCalendar
  • Arch : avoid java.util.Date
  • Arch : avoid java.text.SimpleDateFormat
  • Arch : avoid java.sql.Timestamp
  • Arch : avoid java.sql.Date

to banish jdk dates from your model, you may implement hibernate usertype, jaxb adapter,…

,

6 Comments