Laravelコードを見直す

すみませんが、日本語訳はあとにします。
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](#write_better_laravel_code__chapter1)
* [2. Use Eloquent *when* function](#write_better_laravel_code__chapter2)
* [3. Use Eloquent *updateOrCreate*](#write_better_laravel_code__chapter3)
* [4. Use map instead of bundle if/else](#write_better_laravel_code__chapter4)
* [5. Use collection when you can](#write_better_laravel_code__chapter5)
* [6. Stop calling query in loop](#write_better_laravel_code__chapter6)
* [7. Stop printing raw user inputted values in blade.](#write_better_laravel_code__chapter7)
* [8. Be careful when running javascript with user input.](#write_better_laravel_code__chapter8)
* [9. Stop abusing Morph](#write_better_laravel_code__chapter9)

### 1. <a name="write_better_laravel_code__chapter1" id="write_better_laravel_code__chapter1">Use default params when getting value from request</a>
##### The original code
```php
    $search_type = SearchTypeConstant::TITLE;
    if($request->has('search_type')){
        $search_type = $request->get('search_type');
    }
```
*It is verbose*

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

### <a name="write_better_laravel_code__chapter2" id="write_better_laravel_code__chapter2">2. Use Eloquent *when* function</a>
##### The original code
```php
    $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
```php
    $users = User::active()->when($first_name = $request->get('first_name'), function($first_name){
        return $q->where('first_name', $first_name);
    })->get();
```

### <a name="write_better_laravel_code__chapter3" id="write_better_laravel_code__chapter3">3. Use Eloquent *updateOrCreate*</a>
##### The original code
```php
    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
```php
    $user->profile()->updateOrCreate([], $request->get('profile')->only('phone', 'address'));
```
### <a name="write_better_laravel_code__chapter4" id="write_better_laravel_code__chapter4">4. Use map instead of bundle if/else</a>

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

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

        return 'draft';
    }
```
*It is long*

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

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

### <a name="write_better_laravel_code__chapter5" id="write_better_laravel_code__chapter5">5. Use collection when you can</a>
##### The original code
```php
function get_car_equipment_names($car){
    $names = [];
    foreach($car->equipments as $equipment){
        if($equipment->name){
            $names[] =  $equipment->name;
        }
    }

    return $names;
}
```
*It is long*

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

### <a name="write_better_laravel_code__chapter6" id="write_better_laravel_code__chapter6">6. Stop calling query in loop</a>
Ex1:
##### The original code
```php
    $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
```php
    $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.
```php
    $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
```php
    $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
=>
```php
    $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.
```php
    $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](https://laravel.com/docs/8.x/eloquent-relationships#eager-loading)
In some frameworks, like [phoenix](https://www.phoenixframework.org/), lazy-load is disabled by default to stop incorrect usage.

### <a name="write_better_laravel_code__chapter7" id="write_better_laravel_code__chapter7">7. Stop printing raw user inputted values in blade.</a>
##### The original code
```php
    <div>{!! nl2br($post->comment) !!}</div>
```
*There is a XSS security issue*

#####  Refactoring
```php
    <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.

### <a name="write_better_laravel_code__chapter8" id="write_better_laravel_code__chapter8">8. Be careful when running javascript with user input.</a>
##### The original code
```javascript
    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
```javascript
    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.

### <a name="write_better_laravel_code__chapter9" id="write_better_laravel_code__chapter9">9. Stop abusing Morph</a>
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