Letting the code try different things until it succeeds, neatly

This is the second time I found myself writing this kind of code, and decided that there must be a more readable way to accomplish this:

My code tries to figure something out, that's not exactly well defined, or there are many ways to accomplish it. I want my code to try out several ways to figure it out, until it succeeds, or it runs out of strategies. But I haven't found a way to make this neat and readable.

My particular case: I need to find a particular type of method from an interface. It can be annotated for explicitness, but it can also be the only suitable method around (per its arguments).

So, my code currently reads like so:

Method candidateMethod = getMethodByAnnotation(clazz);
if (candidateMethod == null) {
  candidateMethod = getMethodByBeingOnlyMethod(clazz);
}
if (candidateMethod == null) {
  candidateMethod = getMethodByBeingOnlySuitableMethod(clazz);
}
if (candidateMethod == null) {
  throw new NoSuitableMethodFoundException(clazz);
}

There must be a better way…

Edit: The methods return a method if found, null otherwise. I could switch that to try/catc开发者_运维技巧h logic, but that hardly makes it more readable.

Edit2: Unfortunately, I can accept only one answer :(

To me it is readable and understandable. I'd simply extract the ugly part of the code to a separate method (following some basic principles from "Robert C.Martin: Clean Code") and add some javadoc (and apologies, if necessary) like that:

//...
try {
   Method method = MethodFinder.findMethodIn(clazz);
catch (NoSuitableMethodException oops) {
   // handle exception
}

and later on in MethodFinder.java

/**
 * Will find the most suitable method in the given class or throw an exception if 
 * no such method exists (...)
 */
public static Method findMethodIn(Class<?> clazz) throws NoSuitableMethodException {
  // all your effort to get a method is hidden here,
  // protected with unit tests and no need for anyone to read it 
  // in order to understand the 'main' part of the algorithm.
}

I think for a small set of methods what you're doing is fine.

For a larger set, I might be inclined to build a Chain of Responsibility, which captures the base concept of trying a sequence of things until one works.

I don't think that this is such a bad way of doing it. It is a bit verbose, but it clearly conveys what you are doing, and is easy to change.

Still, if you want to make it more concise, you can wrap the methods getMethod* into a class which implements an interface ("IMethodFinder") or similar:

public interface IMethodFinder{
  public Method findMethod(...);
}

Then you can create instances of you class, put them into a collection and loop over it:

...
Method candidateMethod;
findLoop:
for (IMethodFinder mf: myMethodFinders){
  candidateMethod = mf.findMethod(clazz);
  if (candidateMethod!=null){
    break findLoop;
  }
}

if (candidateMethod!=null){
  // method found
} else {
  // not found :-(
}

While arguably somewhat more complicated, this will be easier to handle if you e.g. need to do more work between calling the findMethods* methods (such as more verification that the method is appropriate), or if the list of ways to find methods is configurable at runtime...

Still, your approach is probably OK as well.

I'm sorry to say, but the method you use seems to be the widely accepted one. I see a lot of code like that in the code base of large libraries like Spring, Maven etc.

However, an alternative would be to introduce a helper interface that can convert from a given input to a given output. Something like this:

public interface Converter<I, O> {
    boolean canConvert(I input);
    O convert(I input);
}

and a helper method

public static <I, O> O getDataFromConverters(
    final I input,
    final Converter<I, O>... converters
){
    O result = null;
    for(final Converter<I, O> converter : converters){
        if(converter.canConvert(input)){
            result = converter.convert(input);
            break;
        }

    }
    return result;
}

So then you could write reusable converters that implement your logic. Each of the converters would have to implement the canConvert(input) method to decide whether it's conversion routines will be used.

Actually: what your request reminds me of is the Try.these(a,b,c) method in Prototype (Javascript).

Usage example for your case:

Let's say you have some beans that have validation methods. There are several strategies to find these validation methods. First we'll check whether this annotation is present on the type:

// retention, target etc. stripped
public @interface ValidationMethod {
    String value();
}

Then we'll check whether there's a method called "validate". To make things easier I assume, that all methods define a single parameter of type Object. You may choose a different pattern. Anyway, here's sample code:

// converter using the annotation
public static final class ValidationMethodAnnotationConverter implements
    Converter<Class<?>, Method>{

    @Override
    public boolean canConvert(final Class<?> input){
        return input.isAnnotationPresent(ValidationMethod.class);
    }

    @Override
    public Method convert(final Class<?> input){
        final String methodName =
            input.getAnnotation(ValidationMethod.class).value();
        try{
            return input.getDeclaredMethod(methodName, Object.class);
        } catch(final Exception e){
            throw new IllegalStateException(e);
        }
    }
}

// converter using the method name convention
public static class MethodNameConventionConverter implements
    Converter<Class<?>, Method>{

    private static final String METHOD_NAME = "validate";

    @Override
    public boolean canConvert(final Class<?> input){
        return findMethod(input) != null;
    }

    private Method findMethod(final Class<?> input){
        try{
            return input.getDeclaredMethod(METHOD_NAME, Object.class);
        } catch(final SecurityException e){
            throw new IllegalStateException(e);
        } catch(final NoSuchMethodException e){
            return null;
        }
    }

    @Override
    public Method convert(final Class<?> input){
        return findMethod(input);
    }

}

// find the validation method on a class using the two above converters
public static Method findValidationMethod(final Class<?> beanClass){

    return getDataFromConverters(beanClass,

        new ValidationMethodAnnotationConverter(),
        new MethodNameConventionConverter()

    );

}

// example bean class with validation method found by annotation
@ValidationMethod("doValidate")
public class BeanA{

    public void doValidate(final Object input){
    }

}

// example bean class with validation method found by convention
public class BeanB{

    public void validate(final Object input){
    }

}

You may use Decorator Design Pattern to accomplish different ways of finding out how to find something.

public interface FindMethod
{
  public Method get(Class clazz);
}

public class FindMethodByAnnotation implements FindMethod
{
  private final FindMethod findMethod;

  public FindMethodByAnnotation(FindMethod findMethod)
  {
    this.findMethod = findMethod;
  }

  private Method findByAnnotation(Class clazz)
  {
    return getMethodByAnnotation(clazz);
  }

  public Method get(Class clazz)
  {
    Method r = null == findMethod ? null : findMethod.get(clazz);
    return r == null ? findByAnnotation(clazz) : r;
  } 
}

public class FindMethodByOnlyMethod implements FindMethod
{
  private final FindMethod findMethod;

  public FindMethodByOnlyMethod(FindMethod findMethod)
  {
    this.findMethod = findMethod;
  }

  private Method findByOnlyMethod(Class clazz)
  {
    return getMethodOnlyMethod(clazz);
  }

  public Method get(Class clazz)
  {
    Method r = null == findMethod ? null : findMethod.get(clazz);
    return r == null ? findByOnlyMethod(clazz) : r;
  } 
}

Usage is quite simple

FindMethod finder = new FindMethodByOnlyMethod(new FindMethodByAnnotation(null));
finder.get(clazz);

... I could switch that to try/catch logic, but that hardly makes it more readable.

Changing the signature of the get... methods so you can use try / catch would be a really bad idea. Exceptions are expensive and should only be used for "exceptional" conditions. And as you say, the code would be less readable.

What is bothering you is the repeating pattern used for flow control--and it should bother you--but there isn't too much to be done about it in Java.

I get really annoyed at repeated code & patterns like this, so for me it would probably be worth it to extract the repeated copy & paste control code and put it in it's own method:

public Method findMethod(Class clazz)
    int i=0;
    Method candidateMethod = null;

    while(candidateMethod == null) {
        switch(i++) {
            case 0:
                candidateMethod = getMethodByAnnotation(clazz);
                break;
            case 1:
                candidateMethod = getMethodByBeingOnlyMethod(clazz);
                break;
            case 2:
                candidateMethod = getMethodByBeingOnlySuitableMethod(clazz);
                break;
            default:
                throw new NoSuitableMethodFoundException(clazz);
    }
    return clazz;
}

Which has the disadvantage of being unconventional and possibly more verbose, but the advantage of not having as much repeated code (less typos) and reads easier because of there being a little less clutter in the "Meat".

Besides, once the logic has been extracted into it's own class, verbose doesn't matter at all, it's clarity for reading/editing and for me this gives that (once you understand what the while loop is doing)

I do have this nasty desire to do this:

case 0:    candidateMethod = getMethodByAnnotation(clazz);                break;
case 1:    candidateMethod = getMethodByBeingOnlyMethod(clazz);           break;
case 2:    candidateMethod = getMethodByBeingOnlySuitableMethod(clazz);   break;
default:   throw new NoSuitableMethodFoundException(clazz);

To highlight what's actually being done (in order), but in Java this is completely unacceptable--you'd actually find it common or preferred in some other languages.

PS. This would be downright elegant (damn I hate that word) in groovy:

actualMethod = getMethodByAnnotation(clazz)                   ?:
               getMethodByBeingOnlyMethod(clazz)              ?:
               getMethodByBeingOnlySuitableMethod(clazz)      ?:
               throw new NoSuitableMethodFoundException(clazz) ;

The elvis operator rules. Note, the last line may not actually work, but it would be a trivial patch if it doesn't.