Ở bài Review automated source code: Với 4 bước để "chạm" tới chất lượng tôi có chia sẻ các tiêu chí review hiệu quả. Tiếp tục trong bài này, tôi xin chia sẻ về những mindset (tư duy) với hy vọng giúp fresher developer nói riêng và developer nói chung có thêm thông tin tham khảo về review source code, để giúp review task là câu chuyện thú vị hơn.
Mỗi công ty có cách thức review source code khác nhau. Mỗi cách thức có điểm hay và điểm chưa hay. Trong phạm vi bài viết này tôi không so sánh các cách thức review này. Nội dung chính là những chia sẻ về trải nghiệm thực tế của tôi tại Cybozu Việt Nam. Tại đây, chúng tôi review theo hình thức “cross-review source code” (Nếu developer này implement thì developer khác trong team review source code).
Bên dưới là 7 tư duy mà tôi đúc kết được sẽ đề cập trong bài này:
#1 Review[1] với mục đích duy nhất giúp source code tốt hơn
#2 Bug là bug, không hạn định bug lớn hay nhỏ khi review
#3 Review là cơ hội cho tất cả mọi người
#4 Review “nghiêm” tạo ra source code “chất”
#5 Reviewer là “người định đoạt” khi không tìm được đồng thuận chung
#6 Review là “chia sẻ” để được “sẻ chia”
#7 Khi review mỗi lần chỉ 50 phút, 400 dòng tối đa
#1 Review[1] với mục đích duy nhất giúp source code tốt hơn
Hồi hộp, lo lắng và đôi khi là “toát mồ hôi” sau khi nhấn enter để gửi comment feedback cho những “developer gạo cội”, trong khi mình là fresher developer. Đó là cảm xúc mà tôi đã trải qua khi chính thức đi làm.
Tôi tin rằng cũng có vài bạn fresher developer mới bước vào nghề cũng có cảm xúc như vậy, ở thời khắc nào đó trên hành trang lập trình của mình.
Những comment review của reviewer mang tính xây dựng, giúp source code tốt hơn nhưng rất dễ bị suy diễn thành hàm ý: trình độ code của developer còn non kinh nghiệm, còn kém, v.v. Đây có lẽ là chuyện không hiếm gặp, nhất là đối với developer nhiều tuổi, developer có vị trí hay developer có cái tôi lớn.
Không sợ làm mất lòng người khác, cứ review trên tinh thần “Review với mục đích duy nhất giúp source code tốt hơn”. Theo thời gian cùng với sự chân thành của mình, tôi tin rằng thông điệp tích cực sẽ truyền được đến họ. Từ đó, chúng ta sẽ hiểu nhau hơn và cùng nhau vượt qua những task “căng” một cách thuận lợi.
Review không phải là cơ hội để chỉ trích cá nhân hoặc đánh bóng bản thân. Tôi tin rằng rất hiếm developer có suy nghĩ này. Hy vọng! Vì một thế giới tốt đẹp.
Đối với developer kinh nghiệm, việc họ viết code chuẩn, hợp nguyên lý hay vận dụng linh hoạt design patterns là điều dễ hiểu.
Tuy nhiên, công nghệ kỹ thuật mới luôn ra đời tính trên đơn vị giây, do đó code mà developer “già” implement ra có thể được cải thiện bằng kỹ thuật mới thông qua comment feedback của fresher developer là điều bình thường. Vì fresher developer thường được tiếp cận nhiều công nghệ mới, hợp thời đại.
Qua đây, xin gửi đến developer thông điệp: Review không phải là cơ hội để đánh giá kỹ năng của người khác mà đơn giản chỉ giúp cho nhau implement tốt hơn. Quan trọng không kém là chọn cách truyền tải phù hợp với tính cách của developer để tránh hàm ý tiêu cực.
#2 Bug là bug, không hạn định bug lớn hay nhỏ khi review
Tôi đã trải qua cả trăm lần review ở nhiều task khác nhau, từ đơn giản đến phức tạp lên đến cả ngàn dòng code trải dài và rộng khắp project. Thậm chí, review những task mà bản thân tôi không có một chút kinh nghiệm về nó. Có task hoàn thành trọn vẹn thuận lợi, có task gian truân xen lẫn nhiều xúc cảm mãi tận đến bây giờ. Sau quá trình đó tôi nghiệm ra rằng: Bug là bug không hạn định bug lớn hay nhỏ.
Nếu là bug thì tốt nhất fix nó càng sớm càng tốt
Bug thì tốt nhất fix càng sớm càng tốt vì chi phí tỉ lệ thuận với thời gian tồn tại của bug. Hình 1.0 bên dưới minh họa cho điều này:
Hình 1.0: Đồ thị minh họa cho cost tăng lên nếu thời gian fix bug càng trễ
Ngoài ra, lỗi nhỏ còn mắc phải cũng nghiêm trọng không kém mấy so với “big bug” :). Do đó, chúng ta không nên suy nghĩ rằng “đây là lỗi nhỏ” hay “lỗi này khó xảy ra với môi trường khách hàng”. Người tiếp nhận feedback với trường hợp này cũng không nên suy nghĩ “lỗi nhỏ không cần fix”.
Không mong đợi bug được tìm thấy ở lúc review
Trong quá trình review, nếu bug được chúng ta phát hiện thường là những bug tiềm ẩn[2]. Reviewer, developer thường cố gắng diễn đạt nó thành các phân khúc: lỗi nhỏ, lỗi S, A, B, v.v[3]. Từ đó, chúng ta sẽ đưa ra nhận định “won’t fix” hay là “fix”.
Phần lớn reviewer không mong muốn phát hiện bug lúc review. Vì điều này, nó làm gián đoạn quá trình review (reviewer dừng lại, chờ developer fixed sau đó tiếp tục review), cũng như chia nhỏ tiến trình review thành nhiều tiến trình con. Điều này đồng thời cũng làm tăng cost của task này (implement và review lại). Nên developer sau khi implement xong, hãy tự review source code của mình trước khi chuyển sang cross-review.
Bug làm gián đoạn trải nghiệm của khách hàng
Nhiều lỗi nhỏ gộp lại gây ra sự gián đoạn trong quá trình trải nghiệm sản phẩm. Điều này có thể gây mất niềm tin của khách hàng. Dẫn đến khách hàng từ chối upgrade hoặc thậm chí kết thúc hợp đồng với sản phẩm. Nên ngay ở giai đoạn phát triển, nếu đã thừa nhận “đó là bug” thì fix càng sớm càng tốt.
#3 Review là cơ hội cho tất cả mọi người
Đối với reviewer, review là cơ hội tiếp thu kỹ thuật mới, mở rộng hiểu biết kỹ thuật thông qua tình huống thực tế được implement. Nên khi bạn muốn học hỏi thêm một kỹ thuật mới mà có task trong team đang implement thì có thể chủ động nhận review để học hỏi.
Có lẽ đây là cách nhanh nhất để cập nhật kỹ thuật mới, là một điểm trong “On the job training”.
Ví dụ: tôi trích lại một thực tế trong quá trình review về CONSTRAINT như sau:
Hình 2.0: Một comment feedback của reviewer, trường hợp 2 bên khác quan điểm về constraint
Trao đổi cũng như chia sẻ hiểu biết về CONSTRAINT giữa reviewer*[5]* và implementer*[4]* đi vào hồi kết như bên dưới:
Hình 3.0: Reviewer và implementer trao đổi để chia sẻ về CONSTRAINT thông qua chat tool
Như vậy, thông qua một tình huống nhỏ như ví dụ trên cả hai sẽ nhận lại những giá trị lớn (Mở rộng thêm về nguyên lý và ứng dụng constraint trong lập trình). Reviewer cũng hiểu sâu sắc về CONSTRAINT
Cả hai tăng cường giao tiếp giúp hiểu phong cách làm việc của nhau hơn. Về lâu dài, điều này giúp team hoạt động hiệu quả hơn, mang lại nhiều giá trị cho team và cho công ty hơn nữa.
#4 Review “nghiêm” tạo ra source code “chất”
Có thể bạn xuống tinh thần nếu source code của bạn có quá nhiều feedback bởi reviewer, nhưng cũng khá tâm đắc khi bạn nhìn lại code này ở codebase trong tương lai
Software industry nói riêng và những ngành công nghiệp khác nói chung, để tạo ra một sản phẩm tinh xảo cần trải qua nhiều công đoạn. Trong đó, không thể thiếu một quá trình “review nghiêm ngặt” trước khi xuất xưởng.
Review giúp tăng tính readable source code để giai đoạn maintain sản phẩm được thuận lợi. Vòng đời của software: 80% cost là maintain, 20% cost là viết mới. Nên chỉ có review nghiêm túc mới thật sự phán đoán được source code có dễ hiểu hay không? Một dòng code nếu đọc qua chỉ trong nháy mắt đã biết nó làm gì thì nó là dòng code “chất”
Source code chất thường áp dụng design pattern và principle. Điều này giúp gia tăng tính mở rộng của software. Vì software phải luôn cập nhật tính năng mới, cải tiến kỹ thuật, công nghệ cũ bằng những kỹ thuật công nghệ mới.
Developer implement tính năng trải qua áp lực lớn mang tên deadline, nên đôi khi “quên” áp dụng những design phù hợp, hay thậm chí vi phạm nguyên lý SOLID là điều có thể xảy ra. Do đó, điều này cũng là một trong những lý do chúng ta cần phải review source code nghiêm túc giúp không để lại dị thường trong codebase.
Người reviewer thường là người sáng suốt hơn vì họ đọc để hiểu trước khi đi sâu hơn vào thuật toán áp dụng. Nên họ feedback thường chính xác, hay đưa ra đề xuất phù hợp mà đôi khi developer “ừ nhỉ” như vừa bừng tỉnh một điều gì đó.
Ví dụ về sự bất hợp lý khi khai array 1 phần tử và cách sử dụng: test-specs/display-moved-message/test.data.js
const senderAccount = { username: 'message_user_01', password: '1' };
const receiverAccount = [{ username: 'message_user_02', password: '1' }]; // Một sự bất thường nhỏ mà reviewer phát hiện
const messageValue = DataGenerator.generatePlainMessage();
messageValue.sender = senderAccount;
messageValue.receiver = [receiverAccount[0].username]; // Một sự bất thường nhỏ mà reviewer phát hiện
const folderInfo = {
creator: senderAccount,
title: `Important ${randomStr()}`,
memo: `This folder is used for saving the message important ${randomStr()}`,
};
export {senderAccount, message, folderInfo};Mỗi người một phương pháp, cũng như có chiến lược riêng để review nghiêm túc. Không quá quan trọng phương pháp hay chiến lược của ai hay hơn. Đơn giản không có một phương pháp nào điểm 10 cả, chỉ có người khai thác phương pháp đó điểm 10 mà thôi.
Nếu bạn đã có xin hãy kiên định và phát huy nó. Với tôi, khi reivew thường cố gắng để lại 20 comment feedback ở mỗi GitHub PR review.
#5 Reviewer là “người định đoạt” khi không tìm được đồng thuận chung
Nếu bạn thực hiện review task với số lượng đủ nhiều thì có thể bạn đã từng gặp tình huống có comment feedback không được tiếp nhận và apply dù đã cố gắng giải thích, chứng minh hay thậm chí cung cấp prototype.
Thường những comment feedback này thuộc về “improve”. Nếu feedback này trao đổi tranh cãi kéo dài thì tốt nhất chúng ta dừng lại và cân nhắc sử dụng mindset: Trong lúc review thì reviewer là người có quyền định đoạt cuối cùng trên comment feedback đó. Vì sao?
Nếu reviewer đọc source code không hiểu nó làm gì một cách dễ dàng thì source code này ở giai đoạn maintain sẽ rất khó khăn và tốn kém vì cả developer và reviewer nhớ nó rất ít.
Reviewer nhìn source code với tâm thế khoan thai, ít áp lực do đó đề cử họ đưa ra sẽ tốt hơn (tất nhiên đã kiểm chứng hoặc có nguồn tin cậy) so với developer chịu nhiều áp lực trong quá trình implement nên có thể giải pháp, thuật toán chưa tối ưu nhất.
Tạm gác chuyện công việc sang một bên, nếu nhìn nhận review source code dưới một góc độ khác thì reviewer chỉ giúp cho source code của developer TỐT hơn. Do đó developer nếu được, hãy thể hiện thiện chí thay lời cảm ơn thì phù hợp hơn phương án từ chối comment feedback.
Hình 4.0: Ví dụ comment feedback kéo dài và dai dẳng
Hình 4.0 ở trên là một hình huống gặp phải khi tôi review cho developer khác. Tranh luận kéo dài và đã tốn riêng tôi tầm 20h tổng thể để:
- Giải thích vì sao nên thay đổi cũng như căn cứ vào đâu để có sự đề cử thay đổi này
- Thuyết phục: Lập luận dựa trên luồng xử lý của source code. Đồng thời, để tăng tính thuyết phục cần implement code snippet đề cử
- Thảo luận trực tiếp, thảo luận nhiều bên
Khá nhiều thời gian trôi qua và kéo dài nhiều ngày mà không tìm được tiếng nói chung . Cuối cùng mindset này được sử dụng, feedback đã được fix và merged vào codebase.
Không ai mong muốn tình huống tranh luận một điểm nào đó trong review comment feedback xảy ra. Như mindset “#1 Review[1] với mục đích duy nhất giúp source code tốt hơn”, nhưng cũng cần có mindset này khi gặp những tình huống trao đổi kéo dài không tìm được đồng thuận chung.
Hơn nữa, nếu đã hy sinh cost cho cuộc trao đổi phân tích kéo dài thì cũng nên đem lại giá trị cụ thể cho task và giúp mang lại kiến thức cho người tham gia.
#6 Review là “chia sẻ” để được “sẻ chia”
Review là cơ hội để chia sẻ kiến thức mới cho bài toán cũ mà đôi khi developer chưa kịp cập nhật.
Đối diện với áp lực về deadline có thể là áp lực lớn nhất của developer, điều này chi phối đến giải pháp cũng như thuật toán developer áp dụng. Thông thường, developer sẽ chọn cách “an toàn nhất” đảm bảo hoạt động chính xác, đảm bảo deadline.
“Hoạt động chính xác”: Trong lập trình một bài toán có rất nhiều giải pháp có thể áp dụng. Chắc chắn rằng mỗi giải pháp có một ưu điểm cũng như hiệu suất khác nhau.
Reviewer là người ngoài cuộc không chịu áp lực về deadline nên có thể “sáng suốt hơn” khi nhìn vào một ngữ cảnh thực tế.
Từ đó reviewer có thể đề cử một thuật toán/giải pháp tốt hơn mà vẫn đảm bảo hoạt động đúng nghiệp vụ. Thuyết phục bằng code snippet và có thể kiểm chứng nên developer thường try thử và được thuyết phục. Khi developer sử dụng đề xuất từ reviewer cho bài toán cũng là lúc kiến thức mới sẽ vô tình có được :)
Thông qua review, reviewer có cơ hội để trình bày kỹ thuật cũ kỹ/lạc hậu của mình bằng những comment hỏi developer cái mới, cái kỹ thuật lạ mà mình chưa hiểu. Muốn biết phải hỏi, muốn giỏi phải học, có lẽ đúng trong tình huống này.
Khi review nếu có logic nào lạ hoặc khó hiểu, chúng ta sẽ comment hỏi developer. Tôi tin rằng developer sẽ nhiệt tình giải thích. Từ đó giúp reviewer biết thêm cái mới, cái hay để có thể áp dụng trong implement tương lai.
Review là cơ hội để sẻ chia cũng như thấu cảm nỗi vất vả mà developer đồng đội đã đối diện để hoàn thành task của team.
Tính năng mới release đến khách hàng là thành quả chung của tất cả mọi người trong team, trong project. Nhưng nếu trễ deadline, hoặc failed release có lẽ là áp lực khủng khiếp đè lên vai developer. Do đó thông qua review task, reviewer có tâm sẽ thấu hiểu và sẻ chia những vất vả trong suốt thời gian hoàn thành task mà developer đối diện. Nếu được, hãy nói lời cảm ơn chân thành đến developer khi review.
#7 Khi review mỗi lần chỉ 50 phút, 400 dòng tối đa
Thời gian tập trung của con người nói chung có giới hạn nhất định và với mỗi người khác nhau thì có một giới hạn khác nhau. Đó cũng là lý do mỗi tiết học của học sinh/sinh viên được thiết kế 45 phút. Trong lập trình sự tập trung cần cao độ và liên tục. Và trải qua nhiều review task tôi nhận thấy 50 phút cho mỗi lần review là phù hợp.
Càng về phút cuối những phút 50 sự tập trung sẽ giảm dần sẽ dẫn đến chất lượng feedback cũng giảm. Nếu bạn đang review GitHub PR mà có nhiều dòng code (hơn 400 dòng) và trải dài trên nhiều class hoặc module thì càng tốn nhiều năng lượng hơn nữa. Để source code được đảm bảo ít sai sót, chúng ta nên dừng lại sau mỗi 50 phút review và relax tầm 10~15 phút để giải phóng căng thẳng đã tích tụ sau 50 phút, để cơ thể có đủ thời gian tái tạo lại trí óc đã tiêu hao. Sau đó, chung ta tiếp tục quay lại review. Hãy relax đúng nghĩa trong 10 phút.
Khám phá điều mới mẻ khi điểm xuất phát review khác thường.
Review từ trên xuống là cách mà hầu hết reviewer tiến hành, thật tuyệt vời và cũng là điều nên tiếp tục áp dụng. Thử làm khác đi một chút xem liệu có gì mới mẻ không! Sau khi giải lao 10 phút và quay lại review tiếp tục, lúc này ta sẽ review từ dưới lên, bỏ qua logical reasoning, bỏ qua yếu tố chạy được, v.v. để xem liệu source code có thật sự dễ hiểu hay không? Nếu đọc tới đâu và hiểu tới đó thì quả thật chúc mừng developer và reviewer cả hai đều làm việc có “tâm” lẫn có “tầm”.
Kết
Hình 5.0: Suy nghĩ theo hướng của sự việc để tìm ra đâu là điều cần thay đổi để tốt hơn
Review là loại task thú vị. Tận dụng nó sẽ giúp chúng ta gặt hái rất nhiều kiến thức bổ ích. Đôi khi giành ra cả ngày để review để rồi chẳng biết bắt đầu từ đâu. Hay những comment feedback cần lôi cả team vào cuộc tranh luận nảy lửa. Để sau đó, nhiều bên liên quan xem xét để cập nhật vào coding standards v.v. Muôn vàn kỷ niệm với chuỗi ngày mang tên “review code”.
Đừng xuống tinh thần nếu bạn đang gặp phải những phiền muộn liên quan tới review. Theo tôi, va chạm là cơ hội giúp ta cứng cáp hơn, hiểu nhau hơn, cũng là tiền đề để tương lai team làm việc hiệu quả hơn.
Trên con đường lập trình, để vươn tới lý tưởng cá nhân đặt ra có rất nhiều thử thách ta cần vượt qua. Do đó đừng để những “issue” liên quan tới review làm chùn bước. Quan trọng nhất là chúng ta nhìn về hướng của sự việc để tìm ra đâu là điều cần thay đổi để tốt hơn.
Cuối cùng, để review đạt hiệu quả và công việc review thêm thú vị. Chúng ta chia một GitHub PR lớn cần review thành nhiều phase với thời gian phù hợp.
Chú thích
- [1]: Đầy đủ là review source code, trong bài này dùng từ review để rút ngắn và giảm sự lặp lại
- [2]: Lỗi được phân tích cấu trúc, logic của feature. Nó chỉ phát sinh lỗi ở ngữ cảnh nhất định trong lúc thực thi chương trình, mà compiler không thể phát hiện
- [3]: Cybozu sử dụng các ký tự S, A, B, C để phân loại bug tìm được trong sản phẩm, và cũng căn cứ vào đây để có độ ưu tiên fix. Tùy vào tổ chức sẽ có cách gán nhãn cho bug khác nhau.
- [4]: Là developer triển khai mã nguồn, trong bài viết này tôi dùng từ implementer thay thế
- [5]: Là developer thực hiện xem xét kiểm tra mã nguồn, trong bài viết này tôi dùng từ reviewer diễn đạt