Clean Code Series - Part 3: Functions

Tiếp nối series về clean code. Phần 3 của bài viết sẽ trình bày về Functions và các tips, tricks để viết được các Functions clean.

Small!

Xin được quote lời tác giả Steven Robert Martin.

The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that.

Quy tắc đầu tiên và cũng là quy tắc quan trọng nhất mà các bạn nên tuân thủ: as small as possible. Nhưng rốt cuộc thế nào là small cho đủ. Thực chất không hề có một quy tắc nào quy định việc thê nào là một hàm đủ nhỏ. Với tác giả, 4 dòng là một số lượng hợp lý. Hoặc các bạn cũng có thể tham khảo Framgia Coding Convention bởi nếu tôi nhớ không nhầm, thì Framgia Coding Convention quy định một function tối đa là 20 dòng. Với riêng bản thân tôi, qua quá trình làm việc và thử nghiệm, thì từ 4-7 dòng là 1 quãng hợp lý hơn cả. Nó phù hợp với hầu hết ngôn ngữ mà tôi sử dụng (PHP - Laravel, Javascript - VueJS, ReactJS, Python, ...). Ngoài ra cũng phải kể thêm, yếu tố bao nhiêu code line một function cũng phụ thuộc phần nào vào Design Pattern, Structure của hệ thống nữa, nên hãy flexible trong khoảng 5-10 nếu cần.

    public function index()
    {
        $teachers = $this->teacherRepository
            ->orderBy('first_name', 'asc')
            ->orderBy('created_at', 'desc')
            ->where(['is_active' => true])
            ->get(['first_name', 'last_name', 'email', 'id'])
            ->toArray();

        return $this->response($teachers);
    }

public function search(Course $course, CourseClass $courseClass, LessonFilter $filter)
    {
        $filter->setArguments(['class_id' => $courseClass->id]);
        $lessons = $this->lessonRepository->search($filter);

        return $this->response([
            'lessons' => $lessons->toArray(),
            'courseClass' => $courseClass->toArray(),
            'course' => $course->load('university')->toArray(),
        ]);
    }

    getLessons(nextProps) {
        let grade = nextProps.selectedGrade;
        if (grade && grade.lesson_ids) {
            let lessonIds = grade.lesson_ids;
            Helper.showSplashScreen();
            this.requestLessons(lessonIds);
        } else {
            this.setState({lessons: []});
        }
    }

    requestLessons(lessonIds) {
        $.ajax({
                url: 'lessons',
                type: 'post',
                data: {lessonIds},
                success: (lessons) => {
                    this.setState({lessons});
                },
                error: (errors) => {
                    toastr.error(Helper.getFirstError(errors));
                },
                complete: () => {
                    Helper.hideSplashScreen();
                }
        })
    }

Blocks And Indenting

Sử dụng Blocks và Indent sẽ giúp code dễ đọc hơn rất nhiều nếu tận dụng tốt. Bạn có thể tham khảo một vài tài liệu trên màng về coding style, ví dụ như PSR (Php Standard Recommend) cho PHP trong này cũng đã quy định khá rõ về việc xuống dòng hay khoảng cách giữa các tham số, chia block cho code, ... Lưu ý thêm 1 điều ở đây, khi bạn dùng tới Blocks hoặc Break Line để phân chia code, mỗi lần indent là 1 lần code được lùi vào so với dòng ở trên, và trong sách, tác giả có khuyên mức lùi vào này tối đa là 2. Thực chất bạn cũng không phải quá quan tâm đến điều này nếu bạn tuân thủ nguyên tắc Small! 4-7 dòng.

    public function getCourses($university, $grade)
    {
        $query = $this->entity;
        if (get_grade($grade) != 'All') {
            $query = $query->where('grade_id', $grade);
        }

        $course = $query->where('university_id', $university)
            ->with(['lessons' => function ($lessonsQuery) {
                $lessonsQuery->where('lessons.day', '>=', $this->getFirstDayOfMonth())
                    ->where('lessons.day', '<=', $this->getLastDayOfMonth());
                },
            ])
            ->get()
            ->toArray();
        return $course;
    }

Do One Thing

Là một trong số những nguyên tắc khó để thực hiện nhất trong cuốn sách này. Khó ở đây một phần do thói quen, một phần do đặc trưng của riêng ngôn ngữ mà Do One Thing trở nên khá rắc rối.

FUNCTIONS SHOULD DO ONE THING. THEY SHOULD DO IT WELL. THEY SHOULD DO IT ONLY.

Nhưng, thực chất thì không thể có hàm nào là Do One Thing cả mà ta tuân thủ 1 cách tương đối (Lưu ý: xin được nhắc lại lần nữa rằng đây là những suy nghĩ và cách tôi áp dụng từ nguyên tắc mà tác giả nêu ra, chứ không phải định nghĩa chính xác, hay các hướng dẫn mà tác giả nêu ra). Hãy đi vào ví dụ sau đây:

// File LessonsController.php
    public function index()
    {
        $lessonIds = request('lessonIds');
        $lessons = $this->getLessonsFromIds($lessonIds);

        return $this->response($lessons);
    }

    public function getLessonsFromIds($lessonIds)
    {
        $lessons = $this->lessonRepository->getList($lessonIds);

        return $lessons;
    }

Ở function getLessonList() ở trên, tôi thực hiện 3 bước:

1. get $lessonIds từ request
2. get tất cả lessons mà có $lessonIds ở trên, thực hiện thông qua một Abstraction Layer là Repository (Search key word: repository patterm để biết thêm chi tiết)
3. return dữ liệu lessons vừa lấy. Viết đến đây thì nhiều bạn sẽ thắc mắc: function getLessonsFromIds($lessonIds) có thể thu gọn về đúng 1 dòng return $this->lessonRepository->getList($lessonIds);, và thay $lessons = $this->getLessonsFromIds($lessonIds); bằng chính $lessons = $this->lessonRepository->getList($lessonIds); thì sẽ ngắn gọn hơn rất nhiều, lý do tôi phải tách function sẽ được giải thích cặn kẽ hơn ở phần sau. Quay trở lại với Do One Thing, ta thấy 1 function đúng chuẩn này lại Do many things, thực chất, ta ngầm định các thao tác như get params $lessonIds = request('lessonIds');, validate dữ liệu (với if, hoặc toán tử ? :) là các thao tác phụ, với mục tiêu hướng tới vẫn là get tất cả các lessons. Ngoài ra, một bước để kiểm tra xem function có Do One Thing không là kiểm tra xem nó trong function có tác động tới 1 hay nhiều Abstraction Level, ta tạm hiểu ở đây là các layer, ví dụ với MVC thì C, V, M đều là các layer hay Abstraction Level.

One Level of Abstraction per Function

Như đã nhắc tới ở trên, quy tắc này hỗ trợ cho việc Do One Thing. Đó cũng là lý do giải thích cho việc tôi đã làm ở trên, chia function ra và function thứ 2 chỉ có ... 1 dòng. Nhưng nếu đọc lại code, nếu tôi thay 2 function trên thành như thế này:

    public function index()
    {
        $lessonIds = request('lessonIds');
        $lessons = $this->lessonRepository->getList($lessonIds);

        return $this->response($lessons);
    }

Đọc đến phần này, bạn sẽ tự hỏi: lessonRepository là gì đúng không. Trong khi nếu tôi giữ nguyên như trên, 2 function, thì chúng ta sẽ không có câu hỏi này bởi lessonRepository đã bị ẩn đi. Thực chất tôi thấy rất nhiều project, lập trình viên vi phạm nguyên tắc này, mục địch lại là để ... clean code hơn.

// File LessonsController.php
    public function show(Lesson $lesson)
    {
        $lesson->load('courseClass', 'courseClass.course');

        return $this->response($lesson);
    }

Đọc thêm về Laravel Route Implicit Binding để hiểu thêm về đoạn code trên, ở đây tôi đã thực hiện Inject model Lesson vào trong function show, tức là đã sử dụng dụng cả Model trong controller, rõ ràng điều này đã vi phạm cả nguyên tắc One Level of Abstraction per Function lẫn Do One Thing, ngoài ra thì thao tác $lesson->load('courseClass', 'courseClass.course'); cũng hoàn toàn là ở Model (thao tác với database), nhưng việc viết code thế này làm mọi thứ đơn giản (clear & clean thì mình đánh giá như nhau) hơn rất nhiều với việc viết như sau: (Sẽ bỏ đi được cả 1 Abstraction Level và 1 function là show trong repo đó)

// File LessonsController.php
    public function show($lessonId)
    {
        $lesson = $this->lessonRepository->show($lessonId);

        return $this->response($lesson);
    }

Bản thân mình thì ko bài trừ cái này cả, khi mọi thứ đơn giản, mình chọn Route Implicit Binding, còn khi có nhiều thao tác logic, code khoảng 10 dòng mới xong function, mình sẽ quay lại tuân thủ nguyên tắc và tạo thêm 1 Abstraction Level nữa. Lựa chọn là tùy ở bạn thôi.

Reading Code from Top to Bottom: The Stepdown Rule

Đọc code từ trên xuống, muốn vậy, hãy viết code từ trên xuống. Ví dụ ở trên, các bạn có bao giờ thắc mắc, là sao ko để function getLessonsFromIds ở trên còn index ở dưới không. Chúng ta có thói quen đọc mọi thứ từ trên xuống dưới (và cảm thấy dễ dàng, thoải mái hơn nếu được vậy) nên khi tôi sử dụng function getLessonsFromIds bên trong index, điều đầu tiên bạn làm là scroll xuống dưới xem nó đâu để đọc tiếp, rồi mới đọc nốt phần còn lại ở index. Hãy phân cấp function của bạn từ trên xuống dưới để có thể theo dõi dễ dàng nhất có thể:

    public function index()
    {
        $this->subIndex1();
        $this->subIndex2();
        $this->subIndex3();
        $this->subIndex4();
    }

    public function subIndex1()
    {
        $this->functionA();
        $this->functionB();
    }

    public function functionA() {}
    public function functionB() {}
    
    public function subIndex2() {}
    public function subIndex3() {}

Switch Statements

Vâng, ác mộng cho các nguyên tắc ở trên là đây. Thật khó để switch có thể tuân thủ các nguyên tắc trên, trong khi mục tiêu tạo ra nó là để xử lý các thao tác đòi hỏi lựa chọn nhiều và đa dạng. Nhưng không sao, mọi vấn đề đều có giải pháp, và ở đây tác giả cũng đưa ra giải pháp cho chúng ta, nhưng bản thân tôi nghĩ, giải pháp này ... sai. Nên tôi sẽ tự mình đưa ra gợi ý, tức là 1 giải pháp của chính tôi cho các bạn. Ví dụ sau:

// Payroll.java
public Money calculatePay(Employee e)
throws InvalidEmployeeType {
     switch (e.type) {
         case COMMISSIONED:
             return calculateCommissionedPay(e);
         case HOURLY:
             return calculateHourlyPay(e);
         case SALARIED:
        	 return calculateSalariedPay(e);
         default:
         	throw new InvalidEmployeeType(e.type);
     }
 }

Có 1 vài vấn đề với function này:

1. Nó quá lớn, vi phạm Small!
2. Do MANY THINGS
3. Vi phạm Single Responsibility Principle (SRP) trong SOLID. Mỗi class chỉ nên có 1 trách nhiệm, nhiều hơn 1 trách nhiệm sẽ khiến nó có nhiều hơn 1 lý do để thay đổi. Và rõ ràng Payroll mà lại còn phải calculatePay thì không thể Single Responsibility được, đáng nghẽ nên có 1 class khác care phần này.
4. Vi phạm Open Closed Principle (OCP) trong SOLID, cũng đúng thôi, switch mà, mỗi lần Employee thêm 1 type khác, ta sẽ phải sửa đổi function này (cry)
5. Duplicated code. Nếu bạn để ý thì thực chất calculateCommissionedPay, calculateHourlyPay, calculateSalariedPay thực hiện cùng 1 nhiệm vụ mà thôi: tính lương dựa trên type của Employee. Tức là đa phần các function này sẽ phải thực hiện các việc giống nhau, và nó sẽ vi phạm 1 nguyên tắc mà tí nữa ta sẽ nhắc tới Don't Repeat Yourself. Vậy ... giải quyết sao? Đơn giản thôi, tôi tin rằng ai đã từng đọc và hiểu về SOLID đều có cách giải quyết rồi. Ta tạo các class ComissionedEmployee, HourlyEmployee, SalariedEmployee extend Employee, trong đó mỗi class đều có method calculatePay để tính lương cho riêng từng Employee Type.

public abstract class EmployeePayroll {
 public abstract Money calculatePay();
}

class ComissionedEmployeePayroll {
    public Money calculatePay() {
		// implement function here
    };
}

class HourlyEmployeePayroll {
    public Money calculatePay() {
		// implement function here
    };
}
--------

// Payroll.java
public Money calculatePay(EmployeePayroll e)
throws InvalidEmployeeType {
     Money money = e.calculatePay();
     
     return money;
 }

Have No Side Effects.

Đã xác định Do One Thing thì đừng có Side Effects. Side Effects ở đây được hiểu là các ảnh hưởng không mong muốn khi ta thực hiện một function. Ví dụ đơn giản thế này:

var x, y;
function increaseX (x) {
	y = y + 2;
	return x + 1;
} ();

Trong function increaseX(x), mục tiêu của ta là tăng x lên 1, nhưng lại có 1 Side Effects ở đây là tăng y lên 2. Và chắc chắn rằng Side Effects luôn gây ảnh hưởng tới chức năng của function, của hệ thống, làm khó khăn thêm quá trình debug của chúng ta lên khá nhiều. Cách duy nhất để hạn chế nó, chính là tuân thủ 2 quy tắc trên cùng Do One Thing và Small, function chúng ta chỉ tập trung hướng đến thực hiện 1 việc cụ thể thì Side Effects càng khó xuất hiện, và càng Small thì sẽ càng dễ phát hiện nó hơn.

Don't Repeat Yourself

Việc lặp code không thỉ làm phình to lên project của chúng ta, mà nó còn tiềm ẩn nhiều mối nguy khác, khiến chúng ta phải tốn công sửa đổi nếu 1 cái cần sửa. Tôi đã từng code 1 file về hiển thị profile, file này là tách riêng cho Teacher và Admin (2 entity riêng biệt trong hệ thống), mặc dù 2 file này giống nhau đến 90%. Và hậu quả đến ngay lập tức, mỗi lần sửa 1 file, là y như rằng không quên thì sửa nhầm file còn lại, hơn thể nữa ta phải mất công đi sửa 2 lần, 2 function giống hệt nhau. Chúng ta có các nguyên tắc thiết kế database để hạn chế lặp dữ liệu, có OOP để đảm bảo tái sử dụng code, vậy mà tự làm hại chính mình khi lặp lại code. Don't Repeat Yourself

Use Descriptive Names

Sử dụng tên có ý nghĩa là một trong các nguyên tắc đã được nhắc tới ở chương trước Meaningful Names nên mình sẽ đi qua rất nhanh, chỉ xin nhắc 1 điều là các bạn càng tuân thủ các nguyên tắc ở trên Small!, Do One Thing thì sẽ càng dễ dàng cho quy tắc này. Ngoài ra, đừng ngại ngần phải dùng 1 cái tên dài, khoảng 20 chữ là hợp lý cho 1 cái tên, tức là khoảng 4 từ. Và tin tôi đi, dùng 1 cái tên miêu tả đúng mục đích, ý nghĩa của function sẽ tốt hơn rất nhiều là dùng 1 cái tên chẳng ai hiểu và viết vài dòng comments kèm theo. Ở chương sau Comments tôi sẽ nói lại về vấn đề này.

Function Arguments

The ideal number of arguments for a function is zero.

Số tham số hợp lý cho một function là 0, và tối đa là 3. Càng nhiều tham số, việc hiểu và bảo trì function sẽ càng phức tạp hơn. Ngoài ra hiểu ngữ cảnh của function kết hợp với param sẽ làm phức tạp hóa thêm vấn đề. Ngoài ra thì sẽ là ác mộng với các function có nhiều param, mỗi 1 param là 2 cái test. Sẽ thế nào nếu có 4 params?

Còn khá nhiều quy tắc mà tôi không nhắc tới, các bạn có thể tìm kiếm cuốn sách để đọc thêm. Cám ơn đã theo dõi tới tận phần này của bài viết. Hẹn gặp lại ở phần tiếp theo trong series.