1000コードレビュー後に学んだこと

LinkedInの作業中、私の仕事のほとんどはコードレビューでした。 私は何度かいくつかの推奨事項を提示したことが判明したため、チームと共有するリストを作成することにしました。



ここに、私の3(+1ボーナス)の最も一般的なコードレビューの推奨事項を示します。



画像



推奨事項1:問題が発生した場合に例外をスローする



多くの場合、パターンは次のようになります。



List<String> getSearchResults(...) { try { List<String> results = // make REST call to search service return results; } catch (RemoteInvocationException e) { return Collections.emptyList(); } }
      
      







このパターンにより、作業中のモバイルアプリケーションの1つで中断が発生しました。 使用したサーバー側の検索は、例外をスローし始めました。 アプリケーションのサーバーAPIには、上記と同様のコードがいくつかあることが判明しました。 したがって、アプリケーションは200のサーバー応答を受信し、各検索要求に対して空のリストを喜んで示しました。



代わりにAPIが例外をスローした場合、監視システムはすぐに例外を処理して修正します。



例外をキャッチした後に単純に空のオブジェクトを返したくなる場合があります。 Javaの空のオブジェクトの例は、Optional.empty()、null、空のリストです。 null値を返したいケースの1つはURL解析です。 URLを文字列から取得できない場合はnullを返す代わりに、「URLが間違っているのはなぜですか? これは、入力ストリームで修正する必要があるデータの問題ですか?”



空のオブジェクトは、このジョブに適したツールではありません。 状況が例外的な場合、例外をスローする必要があります



推奨事項2:最も具体的なデータ型を使用します。



この推奨事項は、 文字列型プログラミングの代替です。



多くの場合、次のようなコードが表示されます。



 void doOperation(String opType, Data data); // where opType is "insert", "append", or "delete", this should have clearly been an enum String fetchWebsite(String url); // where url is "https://google.com", this should have been an URN String parseId(Input input); // the return type is String but ids are actually Longs like "6345789"
      
      







最も具体的な型を使用すると、多くのエラーが回避され、主にJavaなどの強く型付けされた言語を選択する主な理由になります。



それで、問題は、成功したプログラマーがどのようにして強力な型付けで悪いコードを書くことになりますか? 答え:外の世界はあまり典型的ではないからです。 たとえば、プログラムが行を取得する場所はいくつかあります。







これらのすべての場合、この問題を回避するには次の戦略を使用する必要があります。プログラムの最初と最後で行を処理し、シリアル化します。 以下に例を示します。



 // Step 1: Take a query param representing a company name / member id pair and parse it // example: context=Pair(linkedin,456) Pair<String, Long> companyMember = parseQueryParam("context"); // this should throw an exception if malformed // Step 2: Do all the stuff in your application // MOST if not all of your code should live in this area // Step 3: Convert the parameter back into a String at the very end if necessary String redirectLink = serializeQueryParam("context");
      
      







これにはいくつかの利点があります。 不正なデータはすぐに検出されます。 アプリケーションは事前にエラーを出します。 さらに、1回のデータチェック後にアプリケーション全体を分析するときに、例外をキャッチする必要はありません。 さらに、厳密な型指定は、メソッドのより直感的な署名を意味し、各メソッドに対して多数のjavadocsを記述する必要はありません。



推奨事項3:NullではなくOptionalを使用する



Java 8の最も優れた革新の1つはOptional



クラスですOptional



クラスは、存在する場合と存在しない場合があるオブジェクトです。



些細な質問です:どの例外には独自の頭字語がありますか? 回答:NPEまたはNull Pointer Exception。 これは、最も一般的なJava例外であり、しばしば10億ドルのエラーと呼ばれます



Optional



は、プログラムからNPEを完全に削除できます。 ただし、正しく使用する必要があります。 ここにいくつかの提案があります

Optional



使用:







ボーナスの推奨:可能な場合は「解除」メソッド



次のようなメソッドは避けてください。



 // AVOID: CompletableFuture<T> method(CompletableFuture<S> param); // PREFER: T method(S param); // AVOID: List<T> method(List<S> param); // PREFER: T method(S param); // AVOID: T method(A param1, B param2, Optional<C> param3); // PREFER: T method(A param1, B param2, C param3); T method(A param1, B param2); // This method is clearly doing two things, it should be two methods // The same is true for boolean parameters
      
      







これらの方法の共通点は何ですか? オプション、リスト、タスクなどのコンテナオブジェクトをメソッドパラメータとして使用します。 さらに悪いことに、戻り値の型が同じコンテナの場合(つまり、1つのメソッドパラメーターがOptionalを受け入れ、Optionalを返します)。



なんで?

1) Promise A method(Promise B param)





これは柔軟性に劣りますが、簡単です。



2) A method(B param)







Promise B



がある場合は、最初の方法を使用するか、 .map



を使用して関数を「持ち上げる」ことで2番目の方法を使用できます。 (つまり、 promise.map(method)



)。



ただし、Bしかない場合は、2番目の方法を簡単に使用できますが、1番目の方法は使用できないため、2番目の方法の方がはるかに柔軟なオプションになります。



この手法は、一般的な機能ユーティリティのメソッド「リフト」の反対であるため、「アンリフティング」と呼びます。 この方法でメソッドを書き換えると、呼び出されたときにメソッドがより柔軟でシンプルになります。






この翻訳はEDISON Softwareによってサポートされていました。EDISONSoftwareは、Axxon NextとSureView Immixビデオ監視システムを専門的に統合し 、有用な 原発予防 アプリケーションを作成します



All Articles