Những gì tôi học được từ 1000 code reviews

3638

Dưới đây là 3 gợi ý code review (kèm thêm 1 cái bonus) mà tôi tổng hợp được.

Gợi ý 1: Ném (throw) 1 exception khi gặp vấn đề

Pattern mà tôi hay thấy chính là:

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

Pattern này thực sự không hiệu quả với 1 mobile app mà tôi đã từng làm việc. Search backend tôi đang sử dụng bắt đầu throw exceptions. Tuy nhiên, API server của ứng dụng đã có vài đoạn code tương tự như ở trên. Vì vậy, từ khía cạnh của ứng dụng thì nó đã nhận được 200 response thành công và hiển thị 1 danh sách trống cho mọi truy vấn search.

Nếu thay cho việc API đã throw 1 exception, thì các hệ thống monitor của chúng ta sẽ bắt exception này ngay lập tức và fix nó.

Có nhiều lúc bạn sẽ chỉ muốn trả lại 1 object rỗng sau khi đã bắt được 1 exception. Các ví dụ về các objects trống trong Java là Optional.empty(), null, và danh sách trống. Nơi thường xuyên xảy ra chuyện này chính là URL parsing. Thay vì trả lại null nếu URL không thể parse từ String, hãy tự hỏi mình: “Tại sao URL này lại sai form? Đây có phải là vấn đề data mà chúng ta nên sửa upstream?”

Các objects rỗng không phải là công cụ phù hợp cho việc này. Nếu có ngoại lệ, bạn nên throw 1 exception.

Gợi ý 2: Sử dụng type chuyên biệt nhất có thể

Gợi ý này về căn bản sẽ đối lập stringly typed programming.

Tôi thường xuyên thấy những đoạn code như các ví dụ sau:

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"

Sử dụng type chuyên biệt nhất có thể giúp bạn tránh được rất nhiều bug và về cơ bản là lý do để chọn ngôn ngữ strongly typed như Java ngay từ đầu.

Vì vậy, câu hỏi ở đây là: làm thế nào để các programmer có ý tưởng tốt cuối cùng lại viết ra những đoạn code stringly typed tệ như vậy? Đáp án là vì thế giới bên ngoài không phải strongly typed. Strings có thể đến từ rất nhiều nơi khác nhau như:

  • query và path parameters trong urls
  • JSON
  • databases không hỗ trợ enums
  • các thư viện được viết ra tệ

Trong những trường hợp này, bạn nên sử dụng chiến lược sau: giữ string parsing và quá trình serialization bên rìa program của bạn. Ví dụ:

// 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");

Một số lợi ích mà bạn có thể thấy là: Data bị sai form sẽ được phát hiện ngay lập tức; ứng dụng sẽ nhanh chóng không hoạt động nếu có vấn đề. Bạn cũng không phải tiếp tục “bắt” parsing exceptions toàn bộ ứng dụng khi data đã được xác thực 1 lần. Ngoài ra, strong stypes tạo cho nhiều method signatures descriptive hơn; bạn không cần phải viết nhiều javadocs trong mỗi method.

Gợi ý 3: Sử dụng Optionals thay vì các nulls

Một trong những tính năng tốt nhất của Java 8 là class Optional đại diện cho 1 thực thể có thể tồn tại hoặc không tồn tại 1 cách hợp lý.

Câu hỏi nhỏ đặt ra là: Exception duy nhất để có chữ viết tắt của nó là gì? Câu trả lời: 1 NPE hoặc Null Pointer Exception. Đây chính là exception thông dụng nhất trong Java và đã được nhắc đến như 1 lỗi giá trị tỷ đô

Optional cho phép bạn remove hoàn toàn NPEs ra khỏi program của bạn. Tuy nhiên, cần phải sử dụng Optional đúng cách và đây là vài lời khuyên cho bạn:

  • Bạn không nên chỉ call .get() bất cứ khi nào bạn có Optional để dùng, thay vào đó hãy suy nghĩ cẩn thận về trường hợp Optional không hiện diện và đưa ra 1 giá trị mặc định phù hợp
  • Nếu bạn chưa có giá trị mặc định phù hợp thì các methods như .map() và .flatMap() sẽ giúp bạn trì hoãn quyết định này
  • Nếu 1 thư viện external library trả về null để biểu thị 1 case rỗng thì ngay lập tức sẽ wrap giá trị bằng Optional.ofNullable(). nulls có khuynh hướng “thổi phồng” trong các programs nên tốt nhất là ngăn chúng ngay từ source
  • Sử dụng Optional trong type return của các methods. Lúc này, bạn sẽ không cần phải đọc javadoc để xác định liệu giá trị không hiện diện được không

Gợi ý kèm theo: “Gỡ bỏ” các methods bất cứ khi nào có thể

Bạn nên tránh các methods như thế này:

// 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

Điểm chung của các methods này là gì? Chúng đang sử dụng container objects như Optional, List, hoặc Task trong vai trò là các chỉ số method. Thậm chí còn tệ hơn khi return type cùng loại với container (như 1 param methods sẽ lấy 1 Optional và trả lại 1 Optional).

Tại sao
1) Promise<A> method(Promise<B> param) ít linh hoạt hơn việc chỉ có 2) A method(B param).

Nếu bạn có Promise<B> thì có thể sử dụng 1) hoặc sử dụng 2) bằng cách “gỡ bỏ” hàm đó bằng .map. (như: promise.map(method)).

Tuy nhiên, nếu bạn chỉ có B thì có thể dùng 2) mà không thể dùng 1), điều này giúp cho 2) trở thành lựa chọn linh hoạt hơn.

Nếu thích gọi quá trình này là “unlifting” – “gỡ bỏ” vì nó đối lặp với functional utility method thông dụng là “lift”. Việc áp dụng các rewrites này sẽ giúp methods linh hoạt hơn và dễ sử dụng hơn cho các callers.

Nguồn: TopDev via hackernoon.com

RELATED ARTICLESMORE FROM AUTHOR