6

私がこの種のコードを書いていることに気付いたのはこれが2回目であり、これを達成するためのより読みやすい方法が必要であると判断しました。

私のコードは何かを理解しようとしますが、それは正確に定義されていないか、それを達成するための多くの方法があります。コードが成功するか、戦略がなくなるまで、コードを理解するためのいくつかの方法を試してもらいたいと思います。しかし、私はこれをきちんと読みやすくする方法を見つけていません。

私の特定のケース:インターフェースから特定のタイプのメソッドを見つける必要があります。明確にするために注釈を付けることができますが、(引数ごとに)適切な唯一の方法にすることもできます。

したがって、私のコードは現在次のようになっています。

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

もっと良い方法 があるはずです…</p>

編集:メソッドは、見つかった場合はメソッドを返し、nullそうでない場合はメソッドを返します。これをtry/catchロジックに切り替えることはできますが、読みやすくなることはほとんどありません。

Edit2:残念ながら、私は1つの答えしか受け入れることができません:(

4

7 に答える 7

6

私にとって、それは読みやすく、理解しやすいものです。コードの醜い部分を別のメソッドに抽出し (「Robert C.Martin: Clean Code」のいくつかの基本原則に従って)、次のようにいくつかの 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.
}
于 2010-11-02T08:40:17.767 に答える
4

少数のメソッドの場合、実行していることは問題ないと思います。

より大きなセットの場合、私は責任の連鎖を構築する傾向があるかもしれません。これは、1つが機能するまで一連のことを試すという基本概念を捉えています。

于 2010-11-02T11:22:41.200 に答える
2

このやり方が悪いとは思いません。少し冗長ですが、やっていることを明確に伝え、簡単に変更できます。

それでも、もっと簡潔にしたい場合は、メソッド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* メソッドを呼び出す間にさらに多くの作業を行う必要がある場合 (メソッドが適切であることをさらに検証するなど) や、メソッドを見つける方法のリストがランタイム...

それでも、あなたのアプローチもおそらく問題ありません。

于 2010-11-02T08:08:55.267 に答える
2

申し訳ありませんが、ご使用の方法は広く受け入れられているようです。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)各コンバーターは、その変換ルーチンを使用するかどうかを決定するメソッドを実装する必要があります。

実際、あなたのリクエストで思い出したのは、Prototype (Javascript)のTry.these(a,b,c)メソッドです。


あなたの場合の使用例:

検証メソッドを持ついくつかの 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){
    }

}
于 2010-11-02T08:09:01.717 に答える
1

... それを try/catch ロジックに切り替えることはできますが、読みやすくはなりません。

get... メソッドの署名を変更して、try / catch を使用できるようにすることは、非常に悪い考えです。例外はコストがかかるため、「例外的な」条件にのみ使用する必要があります。そして、あなたが言うように、コードは読みにくくなります。

于 2010-11-02T08:00:44.497 に答える
1

Decorator Design Patternを使用して、何かを見つける方法を見つけるさまざまな方法を実現できます。

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);
于 2010-11-02T08:10:07.910 に答える
0

あなたを悩ませているのは、フロー制御に使用される繰り返しパターンです (それはあなたを悩ませるはずです)。

私はこのようなコードとパターンが繰り返されることに本当にイライラするので、繰り返されるコピーと貼り付けの制御コードを抽出し、それを独自のメソッドに入れることはおそらく価値があるでしょう:

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 ではこれはまったく受け入れられません。実際には、他の言語では一般的であるか、好まれていることがわかります。

PS。これは、Groovy では実にエレガントです (くそー、私はその言葉が嫌いです)。

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

elvis 演算子の規則。最後の行は実際には機能しない可能性がありますが、機能しない場合は簡単なパッチになることに注意してください。

于 2010-11-02T23:25:10.307 に答える