Правильно ли я использую контроллер или данный код должен быть в моделях?

Question

[Максим]

Данный код находится в контроллере , правильно ли это , или нужно создать метод в моделях и передавать туда значения?

public  function  buyProduct($id){
        $product = Product::where('id',$id)->get();
        $userId = Auth::user()->id;
        $sellerUserId = $product->first()->seller_user_id;
        $sellerUserIdBalance = User::where('id',$sellerUserId)->get()->first()->balance;
        $productOwnerId = $product->first()->owner_user_id;
        $productPrice = $product->first()->price;
        $userBalance = User::where('id',$userId)->get()->first()->balance;

        if($userId == $productOwnerId){
            $response = [
                'response' => 'You cant buy your product'
            ];
            return response($response,201);

        }
        elseif ($product->first()->status_id == SOLD){
            $response = [
                'response' => 'This product is sold'
            ];
            return response($response,201);
        }
        elseif ($productPrice >= $userBalance ){
            $response = [
                'response' => 'Not enough money'
            ];
            return response($response,201);
        }
        else{
                Product::where('id', $id)->update(array('owner_user_id' => $userId));
                Product::where('id', $id)->update(array('buyer_user_id' => $userId));
                Product::where('id', $id)->update(array('status_id' => SOLD));
                User::where('id', $userId)->update(array('balance' => $userBalance-$productPrice));
                User::where('id', $sellerUserId)->update(array('balance' => $sellerUserIdBalance+$productPrice));
            }
            return  Product::where('id',$id)->get();
        }

Comments

[JhaoDa]

$product->first()->status_id == SOLDИ что, даже работает?

array('owner_user_id' => $userId)2021-й год заканчивается...

Автор, ты откуда-то копипастишь код, не понимая его?

[Максим]

TheAndrey7,

public  function  buyProduct($id){
        $product = Product::find($id);
        $user = Auth::user();

        if($user->id == $product->owner_user_id){
            return 'You cant buy your product';
        }
        elseif($product->status_id == SOLD){
            return 'This product is sold';
        }
        elseif ($user->balance < $product->price) {
            return 'Not enough money';
        }
        else{
            //some code
        }
    }

Вот так правильнее?

[Максим]

TheAndrey7, дублирование

[jazzus]

Здесь хотябы с кодом разобраться, а потом задумываться куда выносить.

[tukreb]

Владелец магазина через знакомую секретаршу попросил её сына, который учится в школе и начал изучать программирование, поправить за бесплатно баги? Экономика должна быть экономной да?

Просто оставлю здесь https://github.com/adelf/acwa_book_ru

[Максим]

tukreb, спасибо за книгу

Answer 1

[Сергей delphinpro]

public function buyProduct($id){
$product = Product::where('id',$id)->get();

Ну тут как бы можно сразу получать в параметре готовый объект модели.
Но даже если получать по айди, то не лучше ли использовать один из методов группы find..()?
Вывод: Знания ларки околонулевые.

Идем дальше

$userId = Auth::user()->id;
$sellerUserId = $product->first()->seller_user_id;
$sellerUserIdBalance = User::where('id',$sellerUserId)->get()->first()->balance;

То есть мы сначала получаем юзера, берем его айди, потом ищем по этому айди юзера в базе (а ведь мы его уже получили ранее) и получаем снова, чтобы дернуть оттуда баланс.
Вывод: Знания ларки околонулевые.

User::where('id', $sellerUserId)->update(array('balance' => $sellerUserIdBalance+$productPrice));

А тут мы еще раз обращаемся к таблице пользователей. Зачем? Мы уже пару раз получили пользователя...

-------------------

А в целом по вопросу, мне кажется норм.
Именно в контроллере определяем, может ли данный юзер осуществить покупку.

Ну или как вариант, можно закинуть это в миддлвэр. При условии, что данные проверки могут выполняться в саброутах. Если проверка только в этом маршруте, то норм.

Comments

[Максим]

а если я сделал так в контроллере

public  function  buyProduct($id){
        $product = Product::find($id);
        $seller = User::find($product->seller_user_id) ;
        $user = Auth::user();
        
        return $product->changeOwner($product,$seller,$user);
    }

и так в моделе

public function changeOwner($product,$seller,$user)
    {
        if($user->id == $product->owner_user_id){

            return 'You cant buy your product';
        }
        elseif($product->status_id == SOLD){
            return 'This product is sold';
        }
        elseif ($user->balance < $product->price) {
            return 'Not enough money';
        }
        else{
            $product->owner_user_id = $user->id;
            $product->buyer_user_id = $user->id;
            $product->status_id = SOLD;
            $user->balance -= $product->price;
            $seller->balance += $product->price;

            $user->save();
            $seller->save();
            $product->save();
        }

        return $product->load(['category:id,title']);
    }

уже лучше?