Đánh giá mã Thực tiễn tốt nhất

Internet cung cấp nhiều tài liệu về đánh giá mã: về tác động của đánh giá mã đối với văn hóa công ty, đánh giá bảo mật chính thức, hướng dẫn ngắn hơn, danh sách kiểm tra dài hơn, đánh giá nhân bản, lý do thực hiện đánh giá mã ở nơi đầu tiên, thực tiễn tốt nhất, tốt nhất thực tiễn, số liệu thống kê về hiệu quả xem xét mã để bắt lỗi và các ví dụ về đánh giá mã đã sai. Ồ, và tất nhiên là có cả sách nữa. Câu chuyện dài ngắn, bài đăng trên blog này trình bày Palantir siêu nhận về đánh giá mã. Các tổ chức có sự miễn cưỡng văn hóa sâu sắc đối với các đánh giá ngang hàng có thể muốn tham khảo bài luận xuất sắc của Karl E. Wiegers, về Nhân cách hóa Nhận xét ngang hàng trước khi thử làm theo hướng dẫn này.

Bài đăng này được sao chép từ các hướng dẫn thực hành tốt nhất của chuỗi công cụ Chất lượng mã Java, Đường cơ sở và bao gồm các chủ đề sau:

  • Tại sao, cái gì và khi nào nên đánh giá mã
  • Chuẩn bị mã để xem xét
  • Thực hiện đánh giá mã
  • Ví dụ đánh giá mã

Động lực

Chúng tôi thực hiện đánh giá mã (CR) để cải thiện chất lượng mã và hưởng lợi từ các hiệu ứng tích cực đối với văn hóa nhóm và công ty. Ví dụ:

  • Các ủy viên được thúc đẩy bởi khái niệm một nhóm người đánh giá sẽ xem xét yêu cầu thay đổi: người ủy thác có xu hướng làm sạch các đầu lỏng lẻo, củng cố TODO và nói chung là cải thiện cam kết. Công nhận chuyên môn mã hóa thông qua các đồng nghiệp là nguồn tự hào cho nhiều lập trình viên.
  • Chia sẻ kiến ​​thức giúp các nhóm phát triển theo nhiều cách:
    - CR truyền đạt rõ ràng chức năng được thêm / thay đổi / loại bỏ cho các thành viên trong nhóm, người sau đó có thể xây dựng dựa trên công việc đã hoàn thành.
    - Người giao dịch có thể sử dụng một kỹ thuật hoặc thuật toán mà người đánh giá có thể học hỏi. Tổng quát hơn, đánh giá mã giúp nâng cao chất lượng thanh trong toàn tổ chức.
    - Người đánh giá có thể có kiến ​​thức về các kỹ thuật lập trình hoặc cơ sở mã có thể giúp cải thiện hoặc củng cố thay đổi; ví dụ, người khác có thể đồng thời làm việc trên một tính năng hoặc sửa lỗi tương tự.
    - Tương tác và giao tiếp tích cực tăng cường trái phiếu xã hội giữa các thành viên trong nhóm.
  • Tính nhất quán trong một cơ sở mã giúp mã dễ đọc và dễ hiểu hơn, giúp ngăn ngừa lỗi và tạo điều kiện hợp tác giữa các loài nhà phát triển thường xuyên và di trú.
  • Tính dễ đọc của các đoạn mã khó đánh giá đối với tác giả có bộ não con và dễ dàng đánh giá cho người đánh giá không có bối cảnh đầy đủ. Mã dễ đọc có thể tái sử dụng nhiều hơn, không có lỗi và không bị lỗi trong tương lai.
  • Lỗi ngẫu nhiên (ví dụ: lỗi chính tả) cũng như lỗi cấu trúc (ví dụ: mã chết, lỗi logic hoặc thuật toán, mối quan tâm về hiệu suất hoặc kiến ​​trúc) thường dễ dàng phát hiện hơn đối với người đánh giá quan trọng với quan điểm bên ngoài. Các nghiên cứu đã phát hiện ra rằng ngay cả các đánh giá mã ngắn và không chính thức cũng có tác động đáng kể đến chất lượng mã và tần suất lỗi.
  • Tuân thủ và môi trường pháp lý thường yêu cầu đánh giá. CR là một cách tuyệt vời để tránh bẫy an ninh phổ biến. Nếu tính năng hoặc môi trường của bạn có các yêu cầu bảo mật quan trọng, nó sẽ được hưởng lợi từ (và có thể yêu cầu) xem xét bởi các điều khoản bảo mật cục bộ của bạn (hướng dẫn OWASP kèm theo là một ví dụ hay về quy trình).

Những gì để xem xét

Không có câu trả lời đúng vĩnh viễn cho câu hỏi này và mỗi nhóm phát triển nên đồng ý về cách tiếp cận của riêng mình. Một số nhóm thích xem xét mọi thay đổi được sáp nhập vào chi nhánh chính, trong khi các nhóm khác sẽ có ngưỡng tầm thường của phạm vi mà theo đó không cần phải xem xét. Sự đánh đổi là giữa thời gian sử dụng hiệu quả các kỹ sư (cả tác giả và người đánh giá) và duy trì chất lượng mã. Trong một số môi trường pháp lý nhất định, việc xem xét mã có thể được yêu cầu ngay cả đối với những thay đổi nhỏ.

Đánh giá mã là không có lớp: là người cao cấp nhất trong nhóm không ngụ ý rằng mã của bạn không cần xem lại. Ngay cả khi, trong trường hợp hiếm hoi, mã là hoàn hảo, đánh giá cung cấp cơ hội cho cố vấn và cộng tác, và, tối thiểu, đa dạng hóa sự hiểu biết về mã trong cơ sở mã.

Khi nào cần xem lại

Việc đánh giá mã sẽ diễn ra sau khi kiểm tra tự động (kiểm tra, kiểu, CI khác) đã hoàn thành thành công, nhưng trước khi mã hợp nhất với nhánh chính của kho lưu trữ. Chúng tôi thường không thực hiện đánh giá mã chính thức về các thay đổi tổng hợp kể từ lần phát hành trước.

Đối với các thay đổi phức tạp sẽ hợp nhất vào nhánh chính là một đơn vị nhưng quá lớn để phù hợp với một CR hợp lý, hãy xem xét mô hình CR xếp chồng: Tạo một tính năng nhánh chính / tính năng lớn và một số nhánh phụ (ví dụ: tính năng / big-Feature-api, tính năng / thử nghiệm tính năng lớn, v.v.) mà mỗi gói đóng gói một tập hợp con của chức năng và được xem xét mã riêng lẻ theo nhánh tính năng / tính năng lớn. Khi tất cả các nhánh phụ được hợp nhất thành tính năng / tính năng lớn, hãy tạo CR để hợp nhất nhánh sau vào nhánh chính.

Chuẩn bị mã để xem xét

Trách nhiệm của tác giả là phải gửi CR dễ dàng để xem xét để không lãng phí người đánh giá Thời gian và động lực:

  • Phạm vi và kích thước. Các thay đổi nên có phạm vi hẹp, được xác định rõ ràng, khép kín mà chúng bao quát toàn diện. Ví dụ: một thay đổi có thể thực hiện một tính năng mới hoặc sửa lỗi. Những thay đổi ngắn hơn được ưa thích hơn những thay đổi dài hơn. Nếu CR thực hiện các thay đổi đáng kể đối với hơn ~ 5 tệp hoặc mất hơn 1 Lần2 để viết hoặc sẽ mất hơn 20 phút để xem xét, hãy xem xét chia nó thành nhiều CR độc lập. Ví dụ: nhà phát triển có thể gửi một thay đổi xác định API cho một tính năng mới về giao diện và tài liệu và thay đổi thứ hai bổ sung thêm các triển khai cho các giao diện đó.
  • Chỉ gửi CR hoàn chỉnh, tự đánh giá (bằng diff) và CR tự kiểm tra. Để tiết kiệm thời gian của người đánh giá, hãy kiểm tra các thay đổi đã gửi (tức là chạy bộ kiểm tra) và đảm bảo rằng họ vượt qua tất cả các bản dựng cũng như tất cả các kiểm tra và kiểm tra chất lượng mã, cả cục bộ và trên máy chủ CI, trước khi chỉ định người đánh giá.
  • Tái cấu trúc thay đổi không nên thay đổi hành vi; ngược lại, một thay đổi thay đổi hành vi nên tránh tái cấu trúc và thay đổi định dạng mã. Có nhiều lý do tốt cho việc này:
    - Tái cấu trúc các thay đổi thường chạm vào nhiều dòng và tệp và do đó sẽ được xem xét mà ít chú ý hơn. Thay đổi hành vi ngoài ý muốn có thể rò rỉ vào cơ sở mã mà không ai nhận thấy.
    - Thay đổi tái cấu trúc lớn phá vỡ việc hái anh đào, nổi loạn và ma thuật kiểm soát nguồn khác. Rất khó để hoàn tác một thay đổi hành vi đã được giới thiệu như là một phần của cam kết tái cấu trúc trên toàn kho lưu trữ.
    - Thời gian xem xét tốn kém của con người nên dành cho logic chương trình hơn là các kiểu tranh luận, cú pháp hoặc định dạng. Chúng tôi thích giải quyết những người có công cụ tự động như Checkstyle, TSLint, Baseline, Prettier, v.v.

Tin nhắn cam kết

Sau đây là một ví dụ về một thông điệp cam kết tốt theo tiêu chuẩn được trích dẫn rộng rãi:

Tóm tắt viết hoa, ngắn (80 ký tự trở xuống)
Văn bản giải thích chi tiết hơn, nếu cần thiết. Gói nó vào khoảng 120 ký tự hoặc hơn. Trong một số bối cảnh, đầu tiên
dòng được coi là chủ đề của một email và phần còn lại của văn bản là phần thân. Dòng trống ngăn cách
tóm tắt từ cơ thể là rất quan trọng (trừ khi bạn bỏ sót toàn bộ cơ thể); các công cụ như rebase có thể bị lẫn lộn nếu bạn chạy
Hai người bên nhau.
Viết thông điệp cam kết của bạn trong mệnh lệnh: "Sửa lỗi" chứ không phải "Đã sửa lỗi" hoặc "Sửa lỗi". Công ước này phù hợp
với các thông điệp cam kết được tạo bởi các lệnh như git merge và git Revert.
Các đoạn tiếp theo đến sau dòng trống.
- Điểm đạn cũng không sao

Cố gắng mô tả cả những gì cam kết thay đổi và cách thực hiện:

> BAD. Đừng làm điều này.
Biên dịch lại
> Tốt.
Thêm phụ thuộc jcsv để sửa lỗi biên dịch IntelliJ

Tìm người đánh giá

Đó là thông lệ cho người đi làm đề xuất một hoặc hai người đánh giá đã quen thuộc với cơ sở mã. Thông thường, một trong những người đánh giá là trưởng dự án hoặc một kỹ sư cao cấp. Chủ dự án nên xem xét đăng ký vào các dự án của họ để nhận được thông báo về CR mới. Đánh giá mã giữa hơn ba bên thường không hiệu quả hoặc thậm chí phản tác dụng vì các nhà đánh giá khác nhau có thể đề xuất thay đổi mâu thuẫn. Điều này có thể cho thấy sự bất đồng cơ bản về việc triển khai chính xác và nên được giải quyết bên ngoài việc xem xét mã trong một diễn đàn có băng thông cao hơn, ví dụ như trực tiếp hoặc trong một cuộc họp video với tất cả các bên liên quan.

Thực hiện đánh giá mã

Đánh giá mã là một điểm đồng bộ hóa giữa các thành viên nhóm khác nhau và do đó có khả năng ngăn chặn tiến trình. Do đó, các đánh giá mã cần được nhắc nhở (theo thứ tự giờ, không phải ngày), và các thành viên trong nhóm và khách hàng tiềm năng cần nhận thức được cam kết về thời gian và ưu tiên thời gian xem xét phù hợp. Nếu bạn không nghĩ rằng bạn có thể hoàn thành đánh giá kịp thời, vui lòng cho người đi làm biết ngay để họ có thể tìm người khác.

Một đánh giá phải đủ kỹ lưỡng để người đánh giá có thể giải thích sự thay đổi ở mức độ chi tiết hợp lý cho một nhà phát triển khác. Điều này đảm bảo rằng các chi tiết của cơ sở mã được biết đến nhiều hơn một người.

Là một người đánh giá, bạn có trách nhiệm thực thi các tiêu chuẩn mã hóa và duy trì chất lượng thanh. Xem lại mã là một nghệ thuật hơn là một khoa học. Cách duy nhất để học nó là làm điều đó; một người đánh giá có kinh nghiệm nên xem xét đưa những người đánh giá ít kinh nghiệm khác vào những thay đổi của họ và để họ thực hiện đánh giá trước. Giả sử tác giả đã làm theo các hướng dẫn ở trên (đặc biệt là liên quan đến việc tự xem lại và đảm bảo mã chạy), ở đây, một danh sách những điều mà người đánh giá nên chú ý trong đánh giá mã:

Mục đích

  • Liệu mã này có hoàn thành mục đích của tác giả không? Mỗi thay đổi cần có một lý do cụ thể (tính năng mới, cấu trúc lại, sửa lỗi, v.v.). Liệu mã được gửi thực sự hoàn thành mục đích này?
  • Hỏi câu hỏi. Hàm và lớp nên tồn tại vì một lý do. Khi lý do không rõ ràng cho người đánh giá, đây có thể là một dấu hiệu cho thấy mã cần phải được viết lại hoặc hỗ trợ với các nhận xét hoặc kiểm tra.

Thực hiện

  • Hãy suy nghĩ về cách bạn sẽ giải quyết vấn đề. Nếu nó khác nhau, tại sao vậy? Mã của bạn xử lý nhiều trường hợp (cạnh)? Là nó ngắn hơn / dễ dàng hơn / sạch hơn / nhanh hơn / an toàn hơn nhưng chức năng tương đương? Có một số mô hình cơ bản mà bạn phát hiện ra đó là bắt giữ bởi mã hiện tại không?
  • Bạn có thấy tiềm năng cho sự trừu tượng hữu ích? Mã trùng lặp một phần thường chỉ ra rằng một phần chức năng trừu tượng hoặc chung hơn có thể được trích xuất và sau đó được sử dụng lại trong các bối cảnh khác nhau.
  • Hãy suy nghĩ như một kẻ thù, nhưng hãy tốt đẹp về nó. Hãy cố gắng để bắt các tác giả của các chuyên gia khác sử dụng các phím tắt hoặc các trường hợp bị thiếu bằng cách đưa ra các cấu hình / dữ liệu đầu vào có vấn đề phá vỡ mã của họ.
  • Hãy suy nghĩ về các thư viện hoặc mã sản phẩm hiện có. Khi ai đó thực hiện lại chức năng hiện có, thường xuyên hơn không phải vì đơn giản vì họ không biết rằng nó đã tồn tại. Đôi khi, mã hoặc chức năng được sao chép theo mục đích, ví dụ, để tránh phụ thuộc. Trong những trường hợp như vậy, một nhận xét mã có thể làm rõ ý định. Là chức năng giới thiệu đã được cung cấp bởi một thư viện hiện có?
  • Sự thay đổi có tuân theo các mẫu chuẩn không? Các cơ sở mã được thiết lập thường thể hiện các mẫu xung quanh các quy ước đặt tên, phân tách logic chương trình, định nghĩa kiểu dữ liệu, v.v ... Thông thường các thay đổi được thực hiện theo các mẫu hiện có.
  • Sự thay đổi có thêm phụ thuộc thời gian biên dịch hoặc thời gian chạy (đặc biệt là giữa các dự án con) không? Chúng tôi muốn giữ cho các sản phẩm của chúng tôi liên kết lỏng lẻo, với càng ít phụ thuộc càng tốt. Thay đổi phụ thuộc và hệ thống xây dựng nên được xem xét kỹ lưỡng.

Tính dễ đọc và phong cách

  • Hãy suy nghĩ về kinh nghiệm đọc của bạn. Bạn đã nắm bắt các khái niệm trong một khoảng thời gian hợp lý? Là dòng chảy lành mạnh và là tên biến và phương thức dễ theo dõi? Bạn có thể theo dõi qua nhiều tệp hoặc chức năng không? Bạn đã đặt ra bằng cách đặt tên không nhất quán?
  • Mã có tuân thủ các nguyên tắc mã hóa và kiểu mã không? Mã có phù hợp với dự án về kiểu dáng, quy ước API, v.v. không? Như đã đề cập ở trên, chúng tôi muốn giải quyết các cuộc tranh luận về phong cách với công cụ tự động.
  • Mã này có TODO không? TODO chỉ chồng chất mã và trở nên cũ kỹ theo thời gian. Yêu cầu tác giả gửi một vé về các vấn đề GitHub hoặc JIRA và đính kèm số vấn đề vào TODO. Thay đổi mã được đề xuất không được chứa mã nhận xét.

Bảo trì

  • Đọc các bài kiểm tra. Nếu không có bài kiểm tra và nên có, yêu cầu tác giả viết một số. Các tính năng thực sự không thể kiểm chứng là rất hiếm, trong khi việc triển khai các tính năng chưa được kiểm tra là không may. Tự kiểm tra các bài kiểm tra: chúng có bao gồm các trường hợp thú vị không? Họ có đọc được không? CR có bảo hiểm kiểm tra tổng thể thấp hơn không? Hãy nghĩ cách mã này có thể phá vỡ. Tiêu chuẩn phong cách cho các bài kiểm tra thường khác với mã lõi, nhưng vẫn quan trọng.
  • CR này có gây ra rủi ro phá vỡ mã kiểm tra, dàn xếp hoặc kiểm tra tích hợp không? Chúng thường không được kiểm tra như là một phần của kiểm tra trước khi cam kết / hợp nhất, nhưng việc chúng bị hỏng là điều đau đớn đối với mọi người. Những điều cụ thể cần tìm là: loại bỏ các tiện ích hoặc chế độ thử nghiệm, thay đổi cấu hình và thay đổi bố cục / cấu trúc tạo tác.
  • Liệu sự thay đổi này phá vỡ khả năng tương thích ngược? Nếu vậy, bạn có thể hợp nhất thay đổi vào thời điểm này hay nên đẩy nó vào một bản phát hành sau? Phá vỡ có thể bao gồm thay đổi cơ sở dữ liệu hoặc lược đồ, thay đổi API công khai, thay đổi quy trình làm việc của người dùng, v.v.
  • Mã này có cần kiểm tra tích hợp không? Đôi khi, mã có thể được kiểm tra đầy đủ với các thử nghiệm đơn vị, đặc biệt nếu mã tương tác với các hệ thống hoặc cấu hình bên ngoài.
  • Để lại phản hồi về tài liệu cấp mã, nhận xét và thông báo cam kết. Các bình luận dư thừa làm lộn xộn mã, và thông điệp cam kết làm bí ẩn những người đóng góp trong tương lai. Đây không phải là luôn luôn áp dụng, nhưng các bình luận chất lượng và thông điệp cam kết sẽ tự trả tiền cho họ. (Hãy nghĩ về một thời gian bạn thấy một thông điệp hoặc bình luận cam kết xuất sắc, hoặc thực sự khủng khiếp.)
  • Là tài liệu bên ngoài được cập nhật? Nếu dự án của bạn duy trì README, CHANGELOG hoặc tài liệu khác, nó có được cập nhật để phản ánh các thay đổi không? Tài liệu lỗi thời có thể gây nhầm lẫn hơn không, và sẽ tốn kém hơn để sửa nó trong tương lai hơn là cập nhật nó ngay bây giờ.

Đừng quên khen ngợi mã ngắn gọn / dễ đọc / hiệu quả / thanh lịch. Ngược lại, từ chối hoặc từ chối CR không phải là thô lỗ. Nếu thay đổi là dư thừa hoặc không liên quan, hãy từ chối nó với một lời giải thích. Nếu bạn cho rằng nó không được chấp nhận do một hoặc nhiều sai sót chết người, hãy từ chối nó, một lần nữa với một lời giải thích. Đôi khi, kết quả đúng của một CR là trên mạng, hãy để Let làm điều này theo một cách hoàn toàn khác

Hãy tôn trọng những người được đánh giá. Mặc dù tư duy đối nghịch là tiện dụng, nhưng nó không phải là tính năng của bạn và bạn có thể quyết định tất cả các quyết định. Nếu bạn có thể đạt được thỏa thuận với người được đánh giá với mã, hãy chuyển sang giao tiếp theo thời gian thực hoặc tìm kiếm ý kiến ​​thứ ba.

Bảo vệ

Xác minh rằng các điểm cuối API thực hiện ủy quyền và xác thực phù hợp phù hợp với phần còn lại của cơ sở mã. Kiểm tra các điểm yếu phổ biến khác, ví dụ: cấu hình yếu, đầu vào của người dùng độc hại, các sự kiện nhật ký bị thiếu, v.v. Khi nghi ngờ, hãy giới thiệu CR cho chuyên gia bảo mật ứng dụng.

Bình luận: súc tích, thân thiện, hành động

Đánh giá nên được súc tích và viết bằng ngôn ngữ trung tính. Phê bình mã, không phải tác giả. Khi một cái gì đó không rõ ràng, yêu cầu làm rõ hơn là giả định không biết gì. Tránh các đại từ sở hữu, đặc biệt là kết hợp với các đánh giá: Mã của tôi đã hoạt động trước khi thay đổi của bạn, Thay đổi phương pháp của bạn có một lỗi, v.v. Tránh các phán đoán tuyệt đối: Lỗi này không bao giờ có thể làm việc, kết quả luôn luôn sai.

Cố gắng phân biệt giữa các đề xuất (ví dụ: Đề xuất: Phương pháp trích xuất để cải thiện mức độ dễ đọc), các thay đổi bắt buộc (ví dụ: Thêm Thêm @Override,) và các điểm cần thảo luận hoặc làm rõ (ví dụ: Đây có thực sự là hành vi đúng không? vì vậy, vui lòng thêm một nhận xét giải thích logic. Xem xét việc cung cấp các liên kết hoặc con trỏ để giải thích sâu sắc về một vấn đề.

Khi bạn hoàn thành việc đánh giá mã, hãy cho biết mức độ bạn mong muốn tác giả phản hồi ý kiến ​​của bạn và liệu bạn có muốn xem lại CR sau khi các thay đổi đã được thực hiện hay không (ví dụ: Cảm thấy thoải mái hợp nhất sau khi phản hồi với một vài gợi ý nhỏ mà so với, vui lòng xem xét các đề xuất của tôi và cho tôi biết khi nào tôi có thể có một cái nhìn khác.

Trả lời đánh giá

Một phần của mục đích đánh giá mã là cải thiện yêu cầu thay đổi của tác giả; do đó, don lồng bị xúc phạm bởi những gợi ý của người đánh giá của bạn và thực hiện chúng một cách nghiêm túc ngay cả khi bạn không đồng ý. Trả lời mọi bình luận, ngay cả khi đó chỉ là một cách đơn giản, thực hiện một cách đơn giản. AC Giải thích lý do tại sao bạn đưa ra một số quyết định, tại sao một số chức năng tồn tại, v.v. Nếu bạn không thể thỏa thuận với người đánh giá, hãy chuyển sang thực tế- thời gian giao tiếp hoặc tìm kiếm một ý kiến ​​bên ngoài.

Các bản sửa lỗi nên được đẩy đến cùng một nhánh, nhưng trong một cam kết riêng. Squashing cam kết trong quá trình xem xét làm cho người đánh giá khó theo dõi các thay đổi.

Các nhóm khác nhau có chính sách hợp nhất khác nhau: một số nhóm chỉ cho phép chủ sở hữu dự án hợp nhất, trong khi các nhóm khác cho phép người đóng góp hợp nhất sau khi xem xét mã tích cực.

Đánh giá mã trực tiếp

Đối với phần lớn các đánh giá mã, các công cụ dựa trên khác biệt không đồng bộ như Có thể xem lại, Gerrit hoặc, GitHub là một lựa chọn tuyệt vời. Các thay đổi phức tạp hoặc đánh giá giữa các bên có chuyên môn hoặc kinh nghiệm rất khác nhau có thể hiệu quả hơn khi được thực hiện trực tiếp, trước cùng một màn hình hoặc máy chiếu hoặc từ xa thông qua VTC hoặc các công cụ chia sẻ màn hình.

Ví dụ

Trong các ví dụ sau, các nhận xét đánh giá được đề xuất được biểu thị bằng // R: ... các nhận xét trong các khối mã.

Đặt tên không nhất quán

lớp MyClass {
  private int CountTotalPageVisits; // R: tên biến
  private int uniqueUsersCount;
}

Chữ ký phương pháp không nhất quán

giao diện MyInterface {
  / ** Trả về {@link Tùy chọn # trống} nếu không thể giải nén s. * /
  công khai tùy chọn  extractString (Chuỗi s);
  / ** Trả về null nếu {@code s} không thể được viết lại. * /
  // R: nên hài hòa các giá trị trả về: cũng sử dụng Tùy chọn <> tại đây
  chuỗi công khai viết lại (Chuỗi s);
}

Sử dụng thư viện

// R: xóa và thay thế bằng MapJoiner của Guava
Chuỗi tham giaAndConcatenate (Bản đồ , Chuỗi keyValueSpayator, Chuỗi keySpayator);

Sở thích cá nhân

int dayCount; // R: nit: Tôi thường thích numFoo hơn fooCount; tùy thuộc vào bạn, nhưng chúng ta nên giữ nó nhất quán trong dự án này

Lỗi

// R: Điều này thực hiện numIterations + 1 lần lặp, đó có phải là cố ý không?
// Nếu có, hãy xem xét thay đổi ngữ nghĩa numIterations?
for (int i = 0; i <= numIterations; ++ i) {
  ...
}

Kiến trúc quan tâm

otherService.call (); // R: Tôi nghĩ chúng ta nên tránh sự phụ thuộc vào OtherService. Chúng ta có thể thảo luận điều này trong người?

Tác giả

Robert F (GitHub / Twitter)