Một vài lỗi comment nên lưu ý trong Rails

Trong thực tế code thì việc bị comment là không thể tránh khỏi. Có nhiều lỗi comment hay gặp là

• comment do sai spec
• comment sai quy ước, tiêu chuẩn
• comment đặt tên
• comment cấu trúc code
• comment sql

1 . Comment sai spec:

Với những lỗi comment liên quan đến spec, sai spec thì phụ thuộc vào khả năng tìm hiểu và nắm spec của dev. Khi được giao task, tốt nhất nên dành thời gian suy nghĩ kỹ về yêu cầu phải làm, database liên quan, các quan hệ giữa các bảng và nhanh chóng raise thắc mắc sớm để khách hàng trả lời rõ ràng. Bởi vì, nếu không clear spec, thì về sau cũng bị bug và mất công sửa lại, ngoài ra cũng ảnh hưởng uy tín với khách hàng.

Về việc tìm hiểu database cũng rất quan trọng. Nên làm rõ mối quan hệ giữa các bảng, ý nghĩa của các trường trong bảng thì sẽ đảm bảo không bị bị động khi có một yêu cầu mới.

2. Comment sai quy ước, tiêu chuẩn:

Khi bắt đầu code, nên đề ra tiêu chuẩn , quy ước trước. VD, dấu cách, dấu nháy, số dòng trong hàm, ... , những quy ước này có thể định nghĩa trong rubocop và phải chạy lệnh rubocop trước khi gửi pull request, hạn chế các comment không đáng có.

$ bundle exec rubocop --auto-correct --display-cop-names --format simple

lệnh rubocop này sẽ tự động chỉnh sửa các lỗi cơ bản theo quy ước trong rubocop.

3. Comment đặt tên:

Lỗi đặt tên thường là lỗi ít chú ý và hay gặp phải nhất, các dev nhiều khi đặt tên cho có. Điều này sẽ làm cho người đọc code khó hiểu và khó maintain hơn. Với một cái tên tốt, thì không cần phải comment hàm này làm gì? .

Tên class phải là danh từ, phản ánh đúng nhiệm vụ duy nhất của class. VD: class với tên là UpdateObjectStatus, tên class này bắt đầu bằng động từ là sai, và cũng không nên dùng tên Object, vì đấy là khái niệm trừu tượng, tên class nên là một đối tượng cụ thể nào đó . Có thể bổ sung hậu tố updater, writer, register khi diễn tả ai update, ai ghi, ai đăng ký , hay thêm cleaner, retriever khi cần xóa hay truy xuất nội dung gì đó. Với VD trên, thể đặt là OrdersStatusUpdater chẳng hạn.

Với tên method, sử dụng danh từ nếu method trả về một vật, sử dụng động từ nếu method trả về một hành động hay thủ tục nào đó VD, ta có tên method update_order_attributes

def update_order_attributes
{
    order_status: 1,
    price: 1000,
    ....
}
end

hàm trên có mục đích là trả về các thuộc tính để update đối tượng order, tuy nhiên tên hàm lại chỉ hành động update order, do đó, tên hàm không phản ánh đúng nội dung hàm thực hiện Ta có thể thay bằng order_attributes_to_update, tên này tuy dài nhưng rõ nghĩa, theo đúng chuẩn nên người đọc có thể nhanh chóng hiểu ra mục đích hàm làm gì mà chưa cần phải đọc nội dung.

Ngoài ra, khi đặt tên cũng phải lưu ý tên số nhiều hay số ít, đảm bảo theo đúng chuẩn ngôn ngữ.

Tên class hay method là rất quan trọng, chúng ta nên bỏ chút thời gian để định nghĩa tên cho đúng.

4. Comment sai cấu trúc:

Có rất nhiều lỗi liên quan đến cấu trúc, tuy nhiên , trong bài viết này chỉ điểm một vài lỗi sau

• Khi khai báo instance_varible cho class thông qua các hàm họ attr_*, nên kiểm tra phạm vi sử dụng của các biến đấy. Nếu không muốn cho các biến này public, thì nên định nghĩa trong private

 class A
 
   private
   attr_reader :b
 end

hoặc khi sử dụng Virtus để định nghĩa attribute trong class , ta nên sử dụng reader: private cho các attributes không cần phải public

class A
   include Virtus.model
   attribute :customer, Customer, reader: :private
end

• Khi định nghĩa constant, nên chú ý đặt constant theo đúng class nhiệm vụ mà constant đấy thể hiện. Nhiều khi, vì muốn code nhanh, nên dev khai báo luôn constant trong class đang dùng, trong khi ý nghĩa của constant này được dùng cho class khác.

• Khi code action controller, nhiều khi chúng ta tùy tiện thêm nhiều private method phục vụ cho action đấy trong controller. Điều này, dẫn tới hậu quả là rất khó kiểm soát được các method tương ứng với action nào, do đó khó khăn trong việc maintain và sửa bug. Lỗi này còn gọi là Fat Controller. Có nhiều cách để làm gầy controller, sử dụng service, hay form, viết module chẳng hạn. Nhưng mục tiêu chung là nhóm các hàm cùng liên quan đến action vào một chỗ để dễ quản lý và mở rộng.

• Nên hạn chế lặp code ít nhất có thể, tuân thủ chặt chẽ theo nguyên tắc DRY, với các method tương tự nhau, nên nhóm và tách để cho dùng chung, với các dòng code có cấu trúc khá giống nhau, chỉ khác tham số thì nên sử dụng các thủ thuật của ruby để gom nhóm lại.

• Comment nhập nhằng về nhiệm vụ của hàm: Hàm private vừa trả về giá trị đồng thời thực hiện một nhiệm vụ khác không liên quan đến kết quả trả về, điều này sẽ dẫn tới người đọc không rõ mục đích của hàm này làm gì. Trong trường hợp này là các method private, thì nên tuân theo quy tắc là mỗi method chỉ phục vụ một nhiệm vụ.

• Comment sử dụng send method: Hàm send trong ruby rất kỳ diệu, cho phép gọi bất kỳ một hàm nào với tên hàm được truyền vào theo dạng string. Tuy nhiên, hàm send có thể gọi cả các private method, điều này thật nguy hiểm, làm ảnh hưởng đến tính đóng gói của class. Chúng ta có thể thay thế bằng method public_send, chỉ cho phép gọi public method

class A
  def method_1
    "call method_1"
  end
  
  private
  def method_2
    "call method_2"
  end
end

a = A.new 
a.send("method_1")  # "call method_1"
a.send("method_2")  # "call method_2"

a.public_send("method_1") # "call method_1"
a.public_send("method_2") # can not call private method 

Ngoài ra, nên hạn chế sử dụng send, hay public_send, vì hàm này cho phép gọi các tên hàm không tồn tại, khi đó kết quả sẽ về nil, và chúng ta sẽ cần có nhiều code để kiểm tra kết quả từ hàm send. do vậy, trước khi gọi hàm send, nên kiểm soát các tham số truyền vào send, giới hạn các tham số trong một mảng tên method nào đó.

• Comment về error code 500 trong API Comment này nằm trong project liên quan đế API. Khi trả về error code cho API, cần lưu ý khi nào thì trả về lỗi 500 Ý nghĩa của lỗi 500 đó là "Theo thiết kế sẽ không xảy ra lỗi này, xảy ra ngoài dự đoán". Vì vậy để dễ tracking sau này thì cứ có lỗi 500 thì nên ra log. Lỗi này xảy ra có nghĩa là bên trong hệ thống đang xảy ra một lỗi nghiêm trọng (ví dụ như request HTTP hoặc liên kết đến DB bị tắc, hoặc là load average nhảy lên cao), do đó để có thể tracking được ngay lập tức thì việc ghi ra log là rất cần thiết.

Ngược lại, đối với các lỗi có thể đoán trước thì không được phép để lỗi 500. Ví dụ như lỗi không lưu được vào DB vì text size quá lớn hoặc parameter type không đúng. (với các lỗi này thì nên dùng lỗi 400 và cần định nghĩa từ trước)

• Sử dụng try không chính xác: Chúng ta đều biết công dụng của hàm try, hay với Rails 5 thì có thể sử dụng & để thay thế

user = User.first 
user.try(:id) == user&.id

Try được sử dụng khi ta đoán đối tượng có thể bị nil khi gọi một hàm khác. Tuy nhiên, trong project về API, nên sử dụng hàm try khi cần thiết. VD, khi user.created_at là 1 optional trong API response, thì có thể sử dụng User.first.try(:created_at) nhưng với user.id là 1 hàm require trong API resonse, thì không sử dụng try

User.first.id 

mặc dù điều này tiềm ẩn lỗi khi User.first là nil, nhưng với yêu cầu của API là required id, tức là nếu theo đúng spec, hay data đúng, luôn luôn có giá trị cho User.first, nếu không có nghĩa là đã có một chỗ xử lý nào đó bị sai. Khi đó, ta cần chương trình raise lỗi để kiểm tra và khắc phục lại.

Khách hàng bên mình đã giải thích là: "Việc handle error có 2 quan điểm đó là Tính chính xác (correctness) và Tính chắc chắn (robustness).

Tính chính xác đó là cách suy nghĩ không được để trả về dữ liệu sai lệch, nếu có khả năng trả về dữ liệu sai lệch thì thà không trả về dữ liệu gì còn hơn.

Tính chắc chắn đó là cách suy nghĩ không làm cho ứng dụng bị dừng, tức là thà trả về dữ liệu sai còn hơn là là ứng dụng bị dừng.

Tuỳ theo từng trường hợp, ta sẽ phải chọn một trong hai cách trên. Thông thường, đối với các phần mềm ví dụ như các GUI application ở bậc cao thì tính chắc chắn cần được ưu tiên, còn đối với các API ở bậc thấp hoặc thao tác với DB thì tính chính xác được ưu tiên (tầm ảnh hưởng của dữ liệu sai là rất lớn dẫn đến việc truy ra dấu vết lỗi trở nên khó khăn).

Trong trường hợp API, spec của API phải được đảm bảo nghiêm ngặt. Nếu có trường hợp mà kết quả trả về có thể là nil, vậy thì phải để là optional."

Do vậy, việc sử dụng try nên được lưu ý.

5. Comment sql:

• Query N+1, sử dụng includes trong query Điều này có thể là bình thường, nhưng nhiều khi code ta ít chú ý từ ban đầu. Chỉ khi đến phase cải thiện performance thì mới quay lại sửa. Vậy nên, khi làm task từ ban đầu, luôn phải lưu ý hạn chế số lượng query nhất có thể.

• Sử dụng update_all hay update_attributes Trừ khi ưu tiên hiệu năng, tốc độ query hay không cần phải quan tâm đến validate, callback thì khi update hàng loạt, ta mới sử dụng hàm update_all, còn lại , nên hạn chế mà sử dụng update_attributes để đảm bảo dữ liệu luôn đúng.

Trên đây là một số lỗi comment mình từng gặp phải, có thể vẫn còn nhiều lỗi comment khác nữa. Và để đảm bảo khi code, hạn chế bị comment nhiều, chúng ta nên lập comment checklist và luôn tự kiểm tra pull request theo checklist này trước khi gửi pull để review.