2
2

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

Laravelコードを見直す

Last updated at Posted at 2021-01-05

すみませんが、日本語訳はあとにします。
After reviewing the code for the new team members, I found long/wrong codes. I write it down here.

Content

1. Use default params when getting value from request

The original code
    $search_type = SearchTypeConstant::TITLE;
    if($request->has('search_type')){
        $search_type = $request->get('search_type');
    }

It is verbose

Refactoring
    $search_type = $request->get('search_type', SearchTypeConstant::TITLE);

2. Use Eloquent when function

The original code
    $query = User::active();
    if($first_name = $request->get('first_name')){
        $query = $query->where('first_name', $first_name);
    }
    $users = $query->get();

We can remove temp $query variable

Refactoring
    $users = User::active()->when($first_name = $request->get('first_name'), function($first_name){
        return $q->where('first_name', $first_name);
    })->get();

3. Use Eloquent updateOrCreate

The original code
    if($user->profile){
        $user->profile->update($request->get('profile')->only('phone', 'address'));
    } else {
        $user->profile()->create($request->get('profile')->only('phone', 'address'));
    }

It is verbose

Refactoring
    $user->profile()->updateOrCreate([], $request->get('profile')->only('phone', 'address'));

4. Use map instead of bundle if/else

The original code
    function get_status_i18n($status){
        if($status == STATUS::COMING){
            return 'coming';
        }

        if($status == STATUS::PUBLISH){
            return 'publish';
        }

        return 'draft';
    }

It is long

Refactoring
    private const MAP_STATUS_I18N = [
        STATUS::COMING => 'coming',
        STATUS::PUBLISH => 'publish'
    ];

    function get_status_i18n($status){
        return self::MAP_STATUS_I18N[$status] ?? 'draft';
    }

5. Use collection when you can

The original code
function get_car_equipment_names($car){
    $names = [];
    foreach($car->equipments as $equipment){
        if($equipment->name){
            $names[] =  $equipment->name;
        }
    }

    return $names;
}

It is long

Refactoring
function get_car_equipment_names($car){
    return $car->equipments()->pluck('name')->filter();
}

6. Stop calling query in loop

Ex1:

The original code
    $books = Book::all(); // *1 query*

    foreach ($books as $book) {
        echo $book->author->name; // Make one query like *select \* from authors where id = ${book.author_id}* every time it is called
    }

N + 1 queries problem

Refactoring
    $books = Book::with('author')->get();
    // Only two queries will be called.
    // select * from books
    // select * from authors where id in (1, 2, 3, 4, 5, ...)

    foreach ($books as $book) {
        echo $book->author->name;
    }
Simple implementation of $books = Book::with('author')->get() code as the bellow.
    $books = Book::all(); //*1 query*
    $books_authors = Author::whereIn('id', $books->pluck('author_id'))->get()->keyBy('author_id'); // *1 query*
    foreach($books as $book){
        $books->author = $books_authors[$book->author_id] ?? null;
    }

Ex2;

The original code
    $books = Book::all(); // *1 query*
    foreach($books as $book){
        echo $book->comments()->count(); // Make one query like *select count(1) from comments where book_id = ${book.id}* every time it is called
    }

N + 1 queries problem

Refactoring

=>

    $books = Book::withCount('comments')->get();
    // Only two queries will be called.
    foreach($books as $book){
        echo $book->comments_count;
    }
Simple implementation of $books = Book::withCount('comments')->get() code as the bellow.
    $books = Book::all(); // *1 query*
    $books_counts= Comment::whereIn('book_id', $books->pluck('id'))->groupBy('book_id')->select(['count(1) as cnt', 'book_id']
    )->pluck('book_id', cnt); // *1 query*

    foreach($books as $book){
        $book->comments_count = $likes[$book->id] ?? 0;
    }

Total: 2 queries

Note: Read more about eager-loading
In some frameworks, like phoenix, lazy-load is disabled by default to stop incorrect usage.

7. Stop printing raw user inputted values in blade.

The original code
    <div>{!! nl2br($post->comment) !!}</div>

There is a XSS security issue

Refactoring
    <div>{{ $post->comment }}</div>

Note: if you want to keep new line in div, you can use white-space: pre-wrap
Rules: Don't use printing raw({!! !}}) with user input values.

8. Be careful when running javascript with user input.

The original code
    function linkify(string){
        return string.replace(/((http|https)?:\/\/[^\s]+)/g, "<a href="%241">$1</a>")
    }

    const $post_content = $("#content_post");
    $post_content.html(linkify($post_content.text()));

There is a XSS security issue.
Easy hack with input is http:deptrai.com<a href="javascript:alert('hacked!');">stackoverflow.com</a> or http:deptrai.com<img src="1" alt="image">

Refactoring
    function linkify(string){
        return string.replace(/((http|https)?:\/\/[^\s]+)/g, "<a href="%241">$1</a>")
    }

    const post = $("#content_post").get(0);
    post.innerHTML = linkify(post.innerHTML)

Bonus: simple sanitize function
Note: Don't use unescaped user input to innerHTML. Almost javascript frameworks will sanitize input before print it into Dom by default.
So, be careful when using some functions like react.dangerouslySetInnerHTML or jquery.append() for user inputted values.

In my test results, WAF(Using our provider's default rules) can prevent server-side XSS quite well, but not good with Dom-Base XSS.

Rules: Be careful when exec scripts that contains user input values.

9. Stop abusing Morph

When using morph, we cannot use foreign key relationship and when the table is grow big we will face performance issues.

The original code
    posts
        id - integer
        name - string

    videos
        id - integer
        name - string

    tags
        id - integer
        name - string

    taggables
        tag_id - integer
        taggable_id - integer
        taggable_type - string
Refactoring
    posts
        id - integer
        name - string

    videos
        id - integer
        name - string

    tags
        id - integer
        name - string

    posts_tags
        tag_id - integer
        post_id - integer

    videos_tags
        tag_id - integer
        video_id - integer

Original post is: https://khoinv.com/post/639439824292593665/write-better-laravel-code

2
2
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
2

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?