views:

323

answers:

2

Recently I worked on FindBugs warnings about exposing internal state, i.e. when a reference to an array was returned instead of returning a copy of the array. I created some templates to make converting that code easier.

Which one did you create to support defensive programming and want to share with the SO crowd?

Templates I've created so far (as examples):

To create a copy of an array to return from a method:

final ${type}[] ${result} = new ${type}[ ${array}.length ];
System.arraycopy( ${array} , 0 , ${result} , 0 , ${array}.length );

To clone an object:

(${o}!= null?(${type})${o}.clone():null)
+2  A: 

Not a template, but I use array.clone() instead of System.arraycopy(). Is there anything wrong with that?

Edit: A template I use when implementing a decorator, especially for an interface with many methods:

wrapped.${enclosing_method}(${enclosing_method_arguments})

It generates an implementation of the current method by delegating the call to a wrapped instance, thus preventing copy/paste errors.

Markus
clone on an array is fine. clone on an untrusted, non-final object is an issue because clone could be overriden to do something nasty/stupid.
Tom Hawtin - tackline
Is this different from Source/Generate Delegate Methods?
Hemal Pandya
So a malicious coded subclass of java.util.Date could override the clone() operation to do some nasty things, right? If so, I should adjust the clone template accordingly... Any suggestions?
dhiller
Wow! I use some of the code generation features, but I just didn't see "Generate delegate methods". I feel stupid now for reinventing the wheel.
Markus
+2  A: 

I like having as a template a "safer" equals() definition:

 /**
 * Implement equals based on ${cursor}. <br />
 * See {@link #compareTo(Object) compareTo}
 * @see java.lang.Object#equals(java.lang.Object)
 */
public boolean equals(final Object anObject)
{
    boolean res = false;
    if(anObject == null) { return false; }
    if(anObject == this) { return true; }
    if(anObject.getClass() == this.getClass())
    {
     res = this.compareTo(anObject) == 0;
    }
    return res;
}

To be sure to always avoid Eq: equals method overrides equals in superclass and may not be symmetric (EQ_OVERRIDING_EQUALS_NOT_SYMMETRIC), where:

This class defines an equals method that overrides an equals method in a superclass. Both equals methods methods use instanceof in the determination of whether two objects are equal.

This is fraught with peril, since it is important that the equals method is symmetrical (in other words, a.equals(b) == b.equals(a)).
If B is a subtype of A, and A's equals method checks that the argument is an instanceof A, and B's equals method checks that the argument is an instanceof B, it is quite likely that the equivalence relation defined by these methods is not symmetric.


This is only for classes implementing Comparable and allows for:

  • an implementation of equals which is always the same;
  • all comparison logics to be located into one place only (the compareTo() function);
  • the compliance with the javadoc of Comparable#compareTo() asking to ensure that (x.compareTo(y)==0) == (x.equals(y)) (strongly recommended, but not strictly required though).
VonC
I would invert the "boolean res" initialization and the class equality test. Then the "else" code can be moved into the "then" clause. I think it makes it easier to understand then and less error prone.
Randy Stegbauer
@Randy: template fixed
VonC
But this template only works if the class implements Comparable, right? Do all your classes implement Comparable? If so, what are the benefits and is the coding-overhead worth it?
dhiller
To expand on what dhiller asks, isn't compareTo bound to call equals() eventually to determine if it should return a positive value, negative, or zero?
matt b
I do use it for my Comparable classes, and deport any comparison logic in the compareTo() method
VonC