Level 73
and you question is?
Dec 20, 2023
3
Level 1
Object-Oriented PHP Code Review for Laravel Compatibility
anyone to help review my code. Please provide the code you'd like me to review, and let me know if there are specific areas or concerns you'd like me to focus on. Additionally, if there are any sentences you'd like me to fix or clarify, please provide them, and I'll do my best to assist you.
<?php
namespace App\Http\Controllers\Admin;
use App\Http\Controllers\Controller;
use App\Http\Resources\CoreResource;
use App\Models\Admin\Customer;
use App\Models\Admin\Player;
use App\Models\Admin\Transactions;
use App\Models\Admin\Transfer;
use App\Models\Admin\User;
use App\Models\Admin\Wallet;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Gate;
use Illuminate\Http\Response;
use Throwable;
class WalletController extends Controller
{
private $transfer;
private $transactions;
private $core;
private $player;
private $admin;
private $user;
private $wallet;
public function __construct(Transfer $transfer, CoreResource $core, Transactions $transactions, Player $player, User $admin, Wallet $wallet)
{
$this->transfer = $transfer;
$this->transactions = $transactions;
$this->core = $core;
$this->admin = $admin;
$this->player = $player;
$this->user = '';
$this->wallet = $wallet;
}
public function tranfer_data(){
abort_if(Gate::denies('Wallet_access'), Response::HTTP_FORBIDDEN, '403 Forbidden');
$response = $this->wallet->tranferData();
return $response;
}
public function person_to_person(){
abort_if(Gate::denies('Wallet_access'), Response::HTTP_FORBIDDEN, '403 Forbidden');
$this->user = auth()->user();
try {
DB::beginTransaction();
if (request()->ajax()) {
$userId = $this->user->id;
$tranferId = request()->id;
$status = request()->status;
$result = $this->transfer->find($tranferId);
$fromUser = $this->player->find($result->from_user);
$toUser = $this->player->find($result->to_user);
$userAvailBalance = $fromUser->wallet->available_balance;
$to_userAvailBalance = $toUser->wallet->available_balance;
$fromUser->lockForUpdate()->get();
$toUser->lockForUpdate()->get();
if ($status == 1) {
if($userAvailBalance>$result->amount){
if ($result) {
$userBalance = $userAvailBalance - $result->amount;
$toUserBalance = $to_userAvailBalance + $result->amount;
$response = $this->transactions->insert([
'user_id' => $result->from_user,
'amount' => $result->amount,
'transaction_id' => $result->transaction_id,
'transaction_type' => $result->transaction_type,
'transaction_method' => $result->transaction_method,
'transaction_mode' => 'dr',
'available_balance' => $userAvailBalance,
'current_balance' => $userBalance,
'status' => 1,
'approved_by' => $result->approved_by,
'created_by' => $userId,
'created_at' => date("Y-m-d H:i:s"),
'updated_at' => date("Y-m-d H:i:s")
]);
$response = $this->transactions->insert([
'user_id' => $result->to_user,
'amount' => $result->amount,
'transaction_id' => $result->transaction_id,
'transaction_type' => $result->transaction_type,
'transaction_method' => $result->transaction_method,
'transaction_mode' => 'cr',
'available_balance' => $to_userAvailBalance,
'current_balance' => $toUserBalance,
'status' => 1,
'approved_by' => $result->approved_by,
'created_by' => $userId,
'created_at' => date("Y-m-d H:i:s"),
'updated_at' => date("Y-m-d H:i:s")
]);
$fromUser->wallet->decrement('available_balance' , $result->amount);
$toUser->wallet->increment('available_balance' , $result->amount);
$result->update(['status' => $status, 'approved_by' => $userId,'available_balance' => $userAvailBalance, 'current_balance' => $userBalance, 'to_user_available_balance' => $to_userAvailBalance, 'to_user_current_balance' => $toUserBalance]);
DB::commit();
if ($response)
return $this->core->message(true, "Transaction Approved !.", 200);
else
return $this->core->message(false, "Something Went Wrong !.", 400);
}
}else{
return $this->core->message(false, "Insufficient Balance!.",422);
}
}else{
$result->update(['status' => $status, 'approved_by' => $userId]);
DB::commit();
return $this->core->message(true, "Transaction Rejected !.", 200);
}
} else {
$transaction = $this->transfer->select('id', 'transaction_id', 'from_user', 'to_user', 'amount', 'available_balance', 'current_balance', 'to_user_available_balance', 'to_user_current_balance', 'status', 'approved_by', 'created_by','created_at', 'updated_at')->where('id', request()->id)->first();
$transaction->requested_at=$this->core->dateFormat($transaction->created_at,'d/m/Y h:i:s A');
$transaction->approved_at=$this->core->dateFormat($transaction->updated_at,'d/m/Y h:i:s A');
$transaction->approved = $this->admin->find($transaction->approved_by);
DB::commit();
return view('admin.wallet.transfer.show', compact('transaction'));
}
} catch (Throwable $th) {
DB::rollBack();
return $this->core->message(false, $th->getMessage(), 400);
}
}
public function amount_deposit()
{
abort_if(Gate::denies('Wallet_access'), Response::HTTP_FORBIDDEN, '403 Forbidden');
try {
DB::beginTransaction();
$this->user = Auth::user();
if (request()->ajax()) {
$transaction = $this->transactions->find(request()->id);
if (request()->status) {
$player = $this->player->find($transaction->user_id);
if($player){
$player->lockForUpdate()->get();
$amount = $transaction->amount;
$available_balance = $player->wallet->available_balance;
$current_balance = $available_balance + $amount;
$player->wallet->increment('available_balance',$amount);
$transaction->update(['available_balance' => $available_balance, 'current_balance' => $current_balance, 'approved_by' => $this->user->id, 'status' => 1]);
DB::commit();
return $this->core->message(true, "Deposit Approved Successfully", 200);
}else{
DB::commit();
return $this->core->message(false, "Player Not Found !", 200);
}
} else {
DB::commit();
$transaction->update(['approved_by' => $this->user->id, 'status' => 0]);
return $this->core->message(true, "Deposit Rejected Successfully", 200);
}
} else {
$transaction = $this->transactions->select('id', 'user_id', 'transaction_id', 'transaction_type', 'transaction_method', 'transaction_mode', 'amount', 'available_balance', 'current_balance', 'status', 'approved_by', 'created_by', 'created_at', 'updated_at')->orderBy('id', 'desc')->where('id', request()->id)->first();
$transaction->requested_at=$this->core->dateFormat($transaction->created_at,'d/m/Y h:i:s A');
$transaction->approved_at=$this->core->dateFormat($transaction->updated_at,'d/m/Y h:i:s A');
$transaction->approved = $this->admin->find($transaction->approved_by);
DB::commit();
return view('admin.wallet.transaction.show', compact('transaction'));
}
} catch (Throwable $th) {
DB::rollBack();
return $this->core->message(false, $th->getMessage(), 400);
}
}
public function amount_withdraw()
{
abort_if(Gate::denies('Wallet_access'), Response::HTTP_FORBIDDEN, '403 Forbidden');
try {
$this->user = Auth::user();
DB::beginTransaction();
if (request()->ajax()) {
$data = $this->transactions->find(request()->id);
if (request()->status) {
$player = $this->player->find($data->user_id);
if($player){
$player->lockForUpdate()->get();
$amount = $data->amount;
$available_balance = $player->wallet->available_balance;
$current_balance = $available_balance - $amount;
$player->wallet->decrement('available_balance' , $amount);
$data->update(['available_balance' => $available_balance, 'current_balance' => $current_balance, 'approved_by' => $this->user->id, 'status' => 1]);
DB::commit();
return $this->core->message(true, "Withdraw Approved Successfully", 200);
}else{
DB::commit();
return $this->core->message(false, "Player Not Found !", 200);
}
} else {
$data->update(['approved_by' => $this->user->id, 'status' => 0]);
DB::commit();
return $this->core->message(true, "Withdraw Rejected Successfully", 200);
}
} else {
$transaction = $this->transactions->select('id', 'user_id', 'transaction_id', 'transaction_type', 'transaction_method', 'transaction_mode', 'amount', 'available_balance', 'current_balance', 'status', 'approved_by', 'created_by', 'created_at', 'updated_at')->orderBy('id', 'desc')->where('id', request()->id)->first();
$transaction->approved = $this->admin->find($transaction->approved_by);
DB::commit();
return view('admin.wallet.transaction.show', compact('transaction'));
}
} catch (Throwable $th) {
DB::rollBack();
return $this->core->message(false, $th->getMessage(), 400);
}
}
}
Level 1
@Tray2 can you review this code and give some suggestions for improve. also this oops concept like construct the model is best way for modern laravel web applications.
Level 73
@leoraj It is quite a bit of a mess. You should break out things to smaller methods, think about the amount of parameters sent to the existing methods. The code is too nested to make sense of at first glance.
You have transactions made after the commit, and you have several commits in each method, the commit could be moved so that you only call it at the end of the method.
$transaction->update(['available_balance' => $available_balance, 'current_balance' => $current_balance, 'approved_by' => $this->user->id, 'status' => 1]);
DB::commit();
return $this->core->message(true, "Deposit Approved Successfully", 200);
}else{
DB::commit();
return $this->core->message(false, "Player Not Found !", 200);
}
} else {
DB::commit();
$transaction->update(['approved_by' => $this->user->id, 'status' => 0]);
return $this->core->message(true, "Deposit Rejected Successfully", 200);
}
The return message could be moved into an enum and then depending on a status you choose the proper response instead of having several returns in you code.
Please or to participate in this conversation.