Позволяя коду пробовать разные вещи, пока он не преуспеет, аккуратно - PullRequest
6 голосов
/ 02 ноября 2010

Я второй раз пишу такой код и решил, что для этого должен быть более читабельный способ:

Мой код пытается выяснить что-то, что не совсем четко определено, или есть много способов сделать это. Я хочу, чтобы мой код попробовал несколько способов выяснить это, пока он не преуспеет или у него не закончатся стратегии. Но я не нашел способа сделать его аккуратным и читабельным.

Мой конкретный случай: мне нужно найти метод определенного типа из интерфейса. Его можно аннотировать для ясности, но он также может быть единственным подходящим методом (согласно его аргументам).

Итак, мой код в настоящее время выглядит так:

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

Там должен быть лучше ...

Редактировать: Методы возвращают метод, если найден, null в противном случае. Я мог бы переключить это на логику try / catch, но это вряд ли сделает ее более читабельной.

Edit2: К сожалению, я могу принять только один ответ: (

Ответы [ 7 ]

6 голосов
/ 02 ноября 2010

Для меня это читабельно и понятно.Я бы просто извлек уродливую часть кода в отдельный метод (следуя некоторым базовым принципам из «Robert C.Martin: Чистый код») и добавил бы некоторый javadoc (и извинения, если необходимо), например:

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

и позже в 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.
}
4 голосов
/ 02 ноября 2010

Я думаю, что для небольшого набора методов то, что вы делаете, хорошо.

Для большего набора я мог бы склониться к созданию Цепочки ответственности , которая захватываетБазовая концепция пробовать последовательность вещей, пока один не работает.

2 голосов
/ 02 ноября 2010

Извините, но метод, который вы используете, кажется общепринятым.Я вижу много такого кода в кодовой базе больших библиотек, таких как Spring, Maven и т. Д.

Однако альтернативой может быть введение вспомогательного интерфейса, который может преобразовывать данные из данного ввода в данный вывод.Примерно так:

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

и вспомогательный метод

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;
}

Итак, вы можете написать конвертеры многократного использования, которые реализуют вашу логику.Каждый из преобразователей должен будет реализовать метод canConvert(input), чтобы решить, будут ли использоваться его процедуры преобразования.

На самом деле: ваш запрос напоминает мне: Try.these (a, b,c) метод в Prototype (Javascript).


Пример использования для вашего случая:

Допустим, у вас есть некоторые bean-компоненты, которые имеют методы проверки.Есть несколько стратегий, чтобы найти эти методы проверки.Сначала мы проверим, присутствует ли эта аннотация в типе:

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

Затем мы проверим, существует ли метод с именем «validate».Чтобы упростить ситуацию, я предполагаю, что все методы определяют один параметр типа Object.Вы можете выбрать другой шаблон.Во всяком случае, вот пример кода:

// 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){
    }

}
2 голосов
/ 02 ноября 2010

Я не думаю, что это такой плохой способ сделать это. Это немного многословно, но оно четко передает то, что вы делаете, и его легко изменить.

Тем не менее, если вы хотите сделать его более кратким, вы можете заключить методы getMethod* в класс, который реализует интерфейс ("IMethodFinder") или аналогичный:

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

Затем вы можете создать экземпляры своего класса, поместить их в коллекцию и выполнить цикл над ней:

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

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

Хотя это, возможно, несколько сложнее, с этим будет легче справиться, если вы, например, нужно больше работать между вызовами методов findMethods * (например, больше проверять, подходит ли метод) или если список способов поиска методов настраивается во время выполнения ...

Тем не менее, ваш подход, вероятно, тоже в порядке.

1 голос
/ 02 ноября 2010

Вы можете использовать Шаблон дизайна декоратора , чтобы выполнить различные способы поиска, как найти что-либо.

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;
  } 
}

Использование довольно просто

FindMethod finder = new FindMethodByOnlyMethod(new FindMethodByAnnotation(null));
finder.get(clazz);
1 голос
/ 02 ноября 2010

... Я мог бы переключить это на логику try / catch, но это вряд ли сделает ее более читаемой.

Изменение подписи методов get ..., чтобы вы могли использовать try / catch, было бы очень плохой идеей. Исключения являются дорогостоящими и должны использоваться только для «исключительных» условий. И, как вы говорите, код будет менее читабельным.

0 голосов
/ 03 ноября 2010

Что вас беспокоит, так это повторяющийся шаблон, используемый для управления потоком - и это должно беспокоить вас - но в Java не так уж много с этим поделать.

Я очень раздраженповторяющийся код и шаблоны, подобные этому, поэтому для меня, вероятно, стоило бы извлечь повторяющийся управляющий код для копирования и вставки и поместить его в свой собственный метод:

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;
}

, который имеет недостаток, заключающийся в том, что он является нестандартным ивозможно, более многословный, но преимущество в том, что в «Мясо» не так много повторяющегося кода (меньше опечаток) и легче читать, потому что в «Мясе» немного меньше помех.

Кроме того, после того, как логика была извлечена вэто собственный класс, многословный вообще не имеет значения, он понятен для чтения / редактирования, и для меня это дает следующее (как только вы поймете, что делает цикл while)

У меня действительно есть это неприятное желание сделать это:

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

Чтобы выделить то, что на самом деле делается (по порядку), но в Java это совершенно неприемлемо - вы на самом деле найдете это common или предпочитаемый в некоторых других языках.

PS.Это было бы совершенно элегантно (черт возьми, я ненавижу это слово) в отличной форме:

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

Оператор elvis правил.Обратите внимание, что последняя строка может на самом деле не работать, но в противном случае это будет тривиальный патч.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...