2

これをもっと読みやすく、きれいにするために何ができるのだろうかと思っています。読みやすいということは、他の開発者にとって読みやすいという意味です。

同じコードを2回使用したくありません。いくつかの方法で短くできると思いますが、よくわかりません...

@Override
public void dispatchEvent(Event event) {
    checkNotNull(event);

    CancellableEvent cancellableEvent = null;
    boolean cancellable;
    if (cancellable = event instanceof CancellableEvent) {
        cancellableEvent = (CancellableEvent) event;
        checkArgument(cancellableEvent.isCancelled());
    }

    // Ignore-cancellation event handlers will run
    for (EventPriority priority : EventPriority.values()) {
        Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, true);
        if (internalMapping != null) {
            for (Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                try {
                    entry.getKey().invoke(entry.getValue(), event);
                } catch (IllegalAccessException e) {
                    e.printStackTrace();
                } catch (IllegalArgumentException e) {
                    e.printStackTrace();
                } catch (InvocationTargetException e) {
                    /*
                     * Delegate any exceptions that occur from
                     * the method to a runtime exception.
                     */
                    throw new RuntimeException(e);
                }
            }
        }
    }

    // Event handlers that consider cancellation will run
    for (EventPriority priority : EventPriority.values()) {
        Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, false);
        if (internalMapping != null) {
            for (Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                try {
                    entry.getKey().invoke(entry.getValue(), event);
                } catch (IllegalAccessException e) {
                    e.printStackTrace();
                } catch (IllegalArgumentException e) {
                    e.printStackTrace();
                } catch (InvocationTargetException e) {
                    /*
                     * Delegate any exceptions that occur from
                     * the method to a runtime exception.
                     */
                    throw new RuntimeException(e);
                }
                // Immediately return in the case of the event being cancelled.
                if (cancellable && cancellableEvent.isCancelled()) {
                    return;
                }
            }
        }
    }
}
4

7 に答える 7

3

例外について話している場合は、Java 7 で例外をクラブできます。

Java7 Exceptionの操作に関する記事はこちら

} catch (ParseException | IOException exception) {
// handle I/O problems.
}

反復については、機能を呼び出すための別のメソッドを持つことができます。

于 2012-09-26T18:48:40.123 に答える
2

いくつかのコードをより読みやすくするために何をお勧めしますか? きれいできれいなコードの基準の 1 つは、長い間知られています。クラスはできるだけ小さくする必要があり、メソッドはできるだけ小さくする必要があります。

これを想定すると、「メソッドの抽出」リファクタリングを実行して、たとえば次のように抽出できます。

processIgnoreCancellationEventHandlers(); processEventHandlersWithPossibleCancellation();

さらに進んで、可能であれば、次のような異なる入力パラメーターを持つ 1 つのメソッドを作成します。

processEventHandlers(noCancellationEventHandlers); processEventHandlers(CancellationAwareEventHandlers);

これにより、次の 2 つの成果が得られます。

  • よりシンプルで短く読みやすいコード、
  • 重複なし。
于 2012-09-26T18:58:21.083 に答える
2

あなたが本当にやりたいことは、これらの 2 つのループをなくすことだと思います。私はそれを力ずくで実行し、必要なすべての引数を含むメソッドを抽出します。次に例を示します。

  @Override
  public void dispatchEvent(Event event) {
      checkNotNull(event);

      CancellableEvent cancellableEvent = null;
      boolean cancellable;
      if (cancellable = event instanceof CancellableEvent) {
          cancellableEvent = (CancellableEvent) event;
          checkArgument(cancellableEvent.isCancelled());
      }

     fireEvents(false, event, cancellableEvent, cancellable);
     fireEvents(true, event, cancellableEvent, cancellable);

  }

  private void fireEvents(boolean considerCancellation, Event event, CancellableEvent cancellableEvent, boolean cancellable)
  {
     // Event handlers that consider cancellation will run
     for (EventPriority priority : EventPriority.values()) {
         Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, ! considerCancellation);
         if (internalMapping != null) {
             for (Map.Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                 try {
                     entry.getKey().invoke(entry.getValue(), event);
                 } catch (IllegalAccessException e) {
                     e.printStackTrace();
                 } catch (IllegalArgumentException e) {
                     e.printStackTrace();
                 } catch (InvocationTargetException e) {
                     /*
                      * Delegate any exceptions that occur from
                      * the method to a runtime exception.
                      */
                     throw new RuntimeException(e);
                 }
                 // Immediately return in the case of the event being cancelled.
                 if ( considerCancellation && cancellable && cancellableEvent.isCancelled()) {
                     return;
                 }
             }
         }
     }
  }

次に、新しい fireEvents メソッドをリファクタリングしてクリーンアップできます。

于 2012-09-26T19:02:26.010 に答える
2

これ以上の文脈がなければ知るのは難しいですが、ここにいくつかの考えがあります.

  • あなたのfor (Entry<Method, EventListener> entry : internalMapping.entrySet()) {ループは両方のループで同じようです。私はそれを独自の方法に入れます。a を取り、Mapループ全体を実行します。そうすれば、2 つの for ループははるかに小さくなります。

    private void runMap(Map<Method, EventListener> methodMap) {
    for (Entry<Method, EventListener> entry : methodMap.entrySet()) {
       ...
    }
}

    その後、1 つのループを実行できます。

    for (EventPriority priority : EventPriority.values()) {
   runMap(getRegistry().getMethodMap(event.getClass(), priority, true));
   runMap(getRegistry().getMethodMap(event.getClass(), priority, false));
}

  • ループ全体を含むループで何かをしている場合if (internalMapping != null) {、私は を使用する傾向がありますif (internalMapper == null) continue;。これにより、インデント レベルが減少します。

  • 例外処理について言及されています。InvocationTargetException最初のものを処理catch (Exception e)してから、残りのすべてを印刷するためにその下に処理することもできます。

于 2012-09-26T18:58:40.677 に答える
2

if 条件で割り当てを行うことはありません。これはエラーが発生しやすいです:

if (cancellable = event instanceof CancellableEvent) {
    ...
}

これを行うだけです：

boolean cancellable = event instanceof CancellableEvent;
if (cancellable) {
    ...
}
于 2012-09-26T19:08:06.147 に答える
1

ループは 1 つのブール値を除いて同一であるため、まずこのように分割し、必要に応じてさらに分割します。

@Override
public void dispatchEvent(Event event) {
    checkNotNull(event);
    CancellableEvent cancellableEvent = null;
    boolean cancellable;
    if (cancellable = event instanceof CancellableEvent) {
        cancellableEvent = (CancellableEvent) event;
        checkArgument(cancellableEvent.isCancelled());
    }
    handleEvents(event, true);
    handleEvents(event, false, cancellableEvent);
}

public void handleEvents(Event event, boolean cancellable)
{
    handleEvents(event, cancellable, null);
}

public void handleEvents(Event event, boolean cancellable, CancellableEvent cancellableEvent)
{
    for (EventPriority priority : EventPriority.values()) {
        Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, cancellable);
        if (internalMapping != null) {
            for (Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                try {
                    entry.getKey().invoke(entry.getValue(), event);
                } catch (IllegalAccessException | IllegalArgumentException e) {
                    e.printStackTrace();
                } catch (InvocationTargetException e) {
                    /*
                    * Delegate any exceptions that occur from
                    * the method to a runtime exception.
                    */
                    throw new RuntimeException(e);
                }
                // Immediately return in the case of the event being cancelled.
                if (cancellableEvent != null && cancellable && cancellableEvent.isCancelled()) {
                    return;
                }
            }
        }
    }
}
于 2012-09-26T19:05:20.270 に答える
1

エントリの呼び出しを別のメソッドにリファクタリングできます。

private final void invokeEntry(Entry<Method, EventListener> entry, Event event) {
    try {
        entry.getKey().invoke(entry.getValue(), event);
    } catch (IllegalAccessException e) {
        e.printStackTrace();
    } catch (IllegalArgumentException e) {
        e.printStackTrace();
    } catch (InvocationTargetException e) {
        /*
         * Delegate any exceptions that occur from
         * the method to a runtime exception.
         */
        throw new RuntimeException(e);
    }
}

次に、 dispatchEventメソッドを次のように置き換えることができます。

@Override
public void dispatchEvent(Event event) {
    checkNotNull(event);

    CancellableEvent cancellableEvent = null;
    boolean cancellable;
    if (cancellable = event instanceof CancellableEvent) {
        cancellableEvent = (CancellableEvent) event;
        checkArgument(cancellableEvent.isCancelled());
    }

    // Ignore-cancellation event handlers will run
    for (EventPriority priority : EventPriority.values()) {
        Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, true);
        if (internalMapping != null) {
            for (Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                invokeEntry(entry, event);
            }
        }
    }

    // Event handlers that consider cancellation will run
    for (EventPriority priority : EventPriority.values()) {
        Map<Method, EventListener> internalMapping = getRegistry().getMethodMap(event.getClass(), priority, false);
        if (internalMapping != null) {
            for (Entry<Method, EventListener> entry : internalMapping.entrySet()) {
                invokeEntry(entry, event);
                // Immediately return in the case of the event being cancelled.
                if (cancellable && cancellableEvent.isCancelled()) {
                    return;
                }
            }
        }
    }
}
于 2012-09-26T18:59:23.920 に答える