2016-09-26 10 views
-1

私は同僚の開発者が作成した次のコードを確認しています。私は、Javaの専門家ではないですが、私見、私は、これは例外の効率的な利用であると感じていなかった - 私が感じる理由はそうです:例外を書くより良い方法 - java

  • 例外をスロー以下のコードは、 - フロー制御のように思えます。 PageErrorRequestAttrUtilクラスに属性エラー属性を追加するだけなので、読みやすくするために例外をスローするのではなく、メソッドを使用する方がよいでしょう。
  • 以下のコードは、実際に呼び出し元に何か問題が発生したことを通知する例外がスローされることが実際に予想されるAPIを定義していません。

私はこれを誤って解釈していることを確認するために2番目の意見を出したかったのです。

以下の最初のクラスは、AEM/Sightly [1]のコンテキストで呼び出され、フロントエンドコード用のJavaバックアップオブジェクトを返します。この例では、AEMページに特定の属性が設定されており、2番目のクラスがリクエストに適合しているかどうかをチェックしています。欠落しているページのユーザー/作成者に役立つメッセージを表示するために使用されます。

[1] https://docs.adobe.com/content/docs/en/aem/6-1/ref/javadoc/com/adobe/cq/sightly/WCMUsePojo.html

クラス1

public class HrefLangUtil extends WCMUsePojo { 
     private static final Logger log = LoggerFactory.getLogger(HrefLangUtil.class); 

     private String hrefLangMultiLine; 
     private List<Map> hrefLangMapList; 
     private String err; 

     final private String COMMA = ","; 
     final private String NEW_LINE = "\\n"; 
     final private static String validUrlCharRegex = "[\\w\\Q!\"#$%&'()*+,-./:;<=>[email protected][\\]^_`{|}~\\E]+"; 
     final private String ERROR_MESSAGE = "The following authored hreflang path and code value pair is invalid: "; 
     final private String HREF_LANG_MULTI_LINE = "hrefLangMultiLine"; 
     final private String HREF_LANG_ERROR_MESSAGE = "hrefLangErrorMessage"; 
     final private String HREF_LANG_PATH = "hrefLangPath"; 
     final private String HREF_LANG_CODE = "hrefLangCode"; 

     @Override 
     @SuppressWarnings("unchecked") 
     public void activate() throws Exception { 
      ValueMap pageProperties = getPageProperties(); 
      hrefLangMultiLine = pageProperties.get(HREF_LANG_MULTI_LINE, StringUtils.EMPTY); 
      if(!hrefLangMultiLine.isEmpty()){ 
       String[] hrefLangLine = hrefLangMultiLine.split(NEW_LINE); 
       hrefLangMapList = new ArrayList<>(); 

       try{ 
        for (String hrefLang : hrefLangLine){ 
         if(!hrefLang.contains(COMMA) || hrefLang.split(COMMA).length > 2 || !hrefLang.matches(validUrlCharRegex)){ 
          throw new Exception(ERROR_MESSAGE + hrefLang); 
         } 
         Map<String, String> hrefLangMap = new HashMap<>(); 
         hrefLangMap.put(HREF_LANG_PATH, hrefLang.split(COMMA)[0]); 
         hrefLangMap.put(HREF_LANG_CODE, hrefLang.split(COMMA)[1]); 
         hrefLangMapList.add(hrefLangMap); 
        } 
       }catch (Exception e){ 
        err = e.getMessage(); 
        log.error(err); 
        SlingHttpServletRequest request = getRequest(); 
        RequestAttr requestAttr = new RequestAttr(request); 
        PageErrorRequestAttrUtil.putPageErrorAttr(HREF_LANG_ERROR_MESSAGE, err, requestAttr); 
       } 
      } 
     } 

     public List<Map> getHrefLangMapList() { 
      return hrefLangMapList; 
     } 

     public String getErr() { 
      return err; 
     } 
    } 

2.クラス2

  public class PageErrorRequestAttrUtil{ 
        static String PAGE_LEVEL_ERROR_MESSAGES = "pageLevelErrorMessages"; 
        private static final Logger log = LoggerFactory.getLogger(PageErrorRequestAttrUtil.class); 

       /** 
       * @param key The key which describes the origin of the error or alert 
       * @param value The actual error message which will be displayed at the top of the page. Should be passed as a {@link String}. 
       * @param requestAttr The object representation {@link RequestAttr} of the the current request 
       * @return String 
       */ 
       @SuppressWarnings("unchecked") 
       public static Object putPageErrorAttr(String key, Object value, RequestAttr requestAttr){ 
        try{ 
         Map<Object, Object> pageLevelErrorMessages; 
         if(!requestAttr.containsKey(key)){ 
          pageLevelErrorMessages = new HashMap<>(); 
          pageLevelErrorMessages.put(key, value); 
          requestAttr.put(PAGE_LEVEL_ERROR_MESSAGES, pageLevelErrorMessages); 
         }else{ 
          if(requestAttr.get(PAGE_LEVEL_ERROR_MESSAGES) instanceof Map){ 
           pageLevelErrorMessages = (Map) requestAttr.get(PAGE_LEVEL_ERROR_MESSAGES); 
           pageLevelErrorMessages.put(key, value); 
           requestAttr.put(key, pageLevelErrorMessages); 
          } else{ 
           throw new Exception(); 
          } 
         } 
        }catch(Exception e){ 
         log.error("The expected pageLevelErrorMessages Map is a reserved key for error messages, it is not of the expected type, Map<>"); 
        } 
        return value; 
       } 
     } 

+0

私はどちらかが好きではありません。エラーロギングは、ブロックが検出されたブロックの内側で行われます。ここには何もスローするべきではありません。 – EJP

+0

フロー制御の例外を使用するのは悪いことです。例外は例外的な条件を対象としており、作成するのに非常にコストがかかります。私は前にこのようなコードを整理しなければならなかった。 –

答えて

0

を提案して下さいはこれのためのより良い場所になります。ご質問に来

次のコードは、実際には、発信者の何かが間違っていた知らせる例外をスローすることが予想されるAPIを定義し、実際にはありません。

activateメソッドが何らかのアクションを実行すると予想され、できないときに例外をスローする可能性があります。メソッドがあったのは別の話だったでしょうcanActivate

以下のコードスロー例外はフロー制御のようです。 PageErrorRequestAttrUtilクラスに属性エラー属性を追加するだけなので、読みやすくするために例外をスローするのではなく、メソッドを使用する方がよいでしょう。

あなたに同意します。上記のコードは簡単に置き換えることができますif...else

関連する問題