Điểm qua một số đoạn code chưa được ổn lắm (Phần 2)

Bài viết dưới đây sẽ tiếp tục chỉ ra một vài trường hợp có thể gặp phải khi viết code PHP nói chung, Laravel nói riêng. Đó có thể là lỗi logic cơ bản, cách sử dụng hàm chưa hợp lý; Hoặc chỉ đơn giản là những lỗi convention không đáng có.

Ngoài ra, một vài ví dụ liên quan đến JS nói chung và Jquery nói riêng cũng sẽ được đề cập.

Phần 1 của series: Điểm qua một số đoạn code chưa được ổn lắm (Phần 1)

10. Sử dụng nhiều hàm isset() cùng lúc

Thay vì viết như sau:

if (isset($a) && isset($b) && isset($c)) {...}

Có thể gộp thành một lời gọi hàm:

if (isset($a, $b, $c) {...}

11. Thứ tự assertion chưa hợp lý khi viết Unit Test

class A {
    function foo() {
        return true;
    }
}

function bar() {
    return new A();
}

$a = bar();
$this->assertTrue($a->foo());
$this->assertInstanceOf(A::class, $a);

Việc kiểm tra $a có phải instance của A::class hay không nên được thực hiện trước khi test các hàm bên trong A::class.

$this->assertInstanceOf(A::class, $a);
$this->assertTrue($a->foo());

12. Dùng hàm isset() để kiểm tra null với cú pháp truy cập mảng trên \Illuminate\Http\Request (Laravel)

public function foo(\Illuminate\Http\Request $request)
{
    if (isset($request['a'])) {
        var_dump('a is exist and not null');
    } else {
        var_dump('a is not exist nor null');
    }
}

Khi gửi request với a = null, hàm trên vẫn sẽ hiển thị kết quả bên trong điều kiện if, thay vì else.

Để có thể truy cập các thuộc tính của đối tượng Request với cú pháp dạng mảng, class Request phải implement interface ArrayAccess.

Do đó, khi sử dụng isset() trên $request['a'], thực chất là đang gọi tới hàm offsetExists được implement trong class \Illuminate\Http\Request .

/**
 * Determine if the given offset exists.
 *
 * @param  string  $offset
 * @return bool
 */
public function offsetExists($offset)
{
    return Arr::has(
        $this->all() + $this->route()->parameters(),
        $offset
);

Hàm này chỉ kiểm tra sự tồn tại của a trong request gửi lên, chứ không kiểm tra giá trị của a có null hay không.

isset($request['a']) === $request->offsetExists('a') // true;

Đây là điểm dễ nhầm lẫn khi hàm isset() mặc định sẽ kiểm tra một biến có tồn tại và khác null hay không.

Vì vậy, hãy thận trọng khi sử dụng cú pháp array trên đối tượng \Illuminate\Http\Request: if (isset($request['a']) && !is_null($request['a'])); hoặc chuyển về dạng array khi sử dụng hàm isset()

public function foo(\Illuminate\Http\Request $request)
{
    $data = $request->only('a');
    if (isset($data['a'])) {
        var_dump('a is exist and not null');
    } else {
        var_dump('a is not exist nor null');
    }
}

13. Dùng magic method vô tội vạ

public function foo(\Illuminate\Http\Request $request)
{
    $title = $request->title;
    $content = $request->content;
    var_dump($title);
    var_dump($content );
}

Đoạn code trên là ví dụ điển hình khi muốn truy cập vào một giá trị trong request gửi lên theo key, dựa vào magic method.

Ở ví dụ này, dữ liệu gửi lên là một request chứa thông tin về tiêu đề (title) và nội dung (content) của một bài post nào đó.

Việc truy cập thông qua key title khá suôn sẻ, mặc dù điều này là không nên và nhiều rủi ro. Có thể tham khảo thêm tại bài viết sau: Laravel requests... DEADLY flexible

Tuy nhiên, vấn đề sẽ thấy rõ hơn khi sử dụng magic method ở dòng code phía dưới:

$content = $request->content;

Dòng code này thay vì lấy giá trị của ô input content gửi lên, thì nó ưu tiên lấy giá trị của thuộc tính protected $content trong class Request trước. Do đó, dẫn đến kết quả không mong muốn.

Chính vì vậy, ngoài việc không đặt tên key trong request trùng với những thuộc tính trong class Request, magic method nên hạn chế được sử dụng khi muốn truy cập vào một giá trị trong Laravel request.

14. Đặt tên hàm không bắt đầu bằng một động từ

Một hàm thường đại diện cho một thao tác/chức năng nào đó. Vì vậy nên đặt tên hàm bắt đầu bằng một động từ.

Thay vì

function passwordValidation() {}

Có thể cân nhắc đổi thành:

function execPasswordValidation() {}

function validatePassword() {}

15. Tên của mảng không ở dạng số nhiều

$post = [];
foreach ($post as $eachPost)
{
    //
}

$commentData = [];
foreach ($commentData as $comment)
{
    //
}

Convention đặt tên mảng ở dạng số nhiều là không bắt buộc trong một số trường hợp. Tuy nhiên, việc đặt tên ở dạng số nhiều sẽ giúp code trở nên dễ đọc hơn đôi chút.

$post = [];
foreach ($posts as $post)
{
    //
}

$comments = [];
foreach ($comments as $comment)
{
    //
}

16. Đặt tên route chưa hợp lý

Route::name('posts.')->prefix('posts')->group(function () {
    Route::get('/', 'PostController@list')->name('list_of_posts');
    // Or
    //Route::get('/', 'PostController@list')->name('post_list');
    //Route::get('/', 'PostController@list')->name('list_post');
    Route::get('/create', 'PostController@create')->name('create_post');
});

Việc gọi và sử dụng route theo route name tương ứng:

route('posts.list_of_posts');
route('posts.create_post');

Thay vào đó, có thể rút gọn lại:

Route::name('posts.')->prefix('posts')->group(function () {
    Route::get('/', 'PostController@list')->name('list');
    Route::get('/create', 'PostController@create')->name('create');
});

route('posts.list');
route('posts.create');

17. Duplicate jQuery selector

$('#foo').show();
$childElements = $('#foo').find('.bar');

Ngoài một số trường hợp đặc biệt, thì đoạn code trên chưa được tối ưu lắm khi thực hiện tìm kiếm cùng một element tới hai lần. Có thể cân nhắc sửa lại như sau:

var parentElement = $('#foo');
parentElement.show();
var childElements = parentElement.find('.bar');

18. Truy cập thuộc tính có thể không tồn tại trong một đối tượng

$.ajax({
    error: function (errorResponse) {
        let errors = errorResponse.foo.bar;
        showBarError(errors);
    },
});

Ở ví dụ trên, lỗi khi submit Ajax request sẽ được hiển thị thông qua hàm showBarError(). Hàm này nhận tham số là giá trị của thuộc tính bar trong đối tượng errorResponse.foo.

Nếu như response lỗi được trả về là một đối tượng có cấu trúc đúng như trên, việc hiển thị lỗi sẽ không gặp vấn đề gì.

Tuy nhiên, trong thực tế, đặc biệt là những trường hợp xử lý lỗi, dữ liệu trả về có thể bất định và khó lường trước. Do đó, nên cân nhắc bổ sung các câu điều kiện kiểm tra trước khi truy cập thuộc tính của một đối tượng hoặc sử dụng cú pháp try-catch.

$.ajax({
    error: function (errorResponse) {
        try {
            let errors = errorResponse.foo.bar;
            showError(errors);
        } catch (exception) {
            showUndefinedError(exception);
        }
    },
});

$.ajax({
    error: function (errorResponse) {
        let errors = [];
        let foo = errorResponse.foo;
        if (foo.bar) {
            errors = foo.bar;
        }

        showBarError(errors);
    },
});

19. Khởi tạo giá trị không cần thiết

let foo = 'bar';

if (isTrue()) {
    foo = 'true';
} else {
    foo = 'false';
}

Việc khởi tạo giá trị ban đầu bar trong ví dụ trên là không cần thiết vì giá trị của biến foo luôn bị ghi đè trong câu lệnh if-else phía dưới. Tùy vào bài toán thực tế, có thể sửa lại theo một trong hai hướng sau:

let foo = 'false';

if (isTrue()) {
    foo = 'true';
}

let foo;

if (isTrue()) {
    foo = 'true';
} else {
    foo = 'false';
}

Tuy nhiên, trong một số trường hợp đặc biệt, ví dụ như hàm isTrue() throw một exception nào đó, việc khởi tạo giá trị ban đầu vẫn trở nên cần thiết.

let foo = 'bar';

try {
    if (isTrue()) {
        foo = 'true';
    } else {
        foo = 'false';
    }
} catch (e) {
    report(e);
}