2
\$\begingroup\$

I'm using PHP/Laravel and I have two controllers.

  • Controllers/PaymentController
  • Controllers/PaymentExpressControler

They both create an request which is exactly the same, but it has some differences.

Let's say, they both use createRequest(Request $Request) which is a POST request to a third-party api.

I'm wondering how can I reduce the code, because it's clearly duplicated.

I was thinking of two options, a trait or a class.

  • Controllers/Payment/(NameOfThirdParty).php which is a class with createRequest

How would this sound like?

My current createRequest

protected function createRequest(Request $request)
    {
        $this->cartCollection();

        $availableMethods = ['mb', 'mbw', 'cc', 'dd'];
        $tkey = uniqid(Auth::id(), true);

        if (in_array($request->type, $availableMethods)) {
            $payment_body = [
                "method" => $request->type,
                "value" => (float)\Cart::getTotal(),
                "currency" => "EUR",
                "key" => $tkey,
                "type" => "sale",
                "customer" =>
                [
                    "name" => Auth::user()->name,
                    "email" => Auth::user()->email,
                    "phone" => isset($request->telm) ? $request->telm : "",
                ],
                "capture" =>
                [
                    "descriptive" => "Order",
                    "transaction_key" => $tkey,
                ],
            ];
            if ($request->type == 'dd') {
                $sdd_mandate =  ['sdd_mandate' => [
                    "iban" => $request->iban,
                    "account_holder" => Auth::user()->name,
                    "name" => Auth::user()->name,
                    "email" => Auth::user()->email,
                    "phone" => isset($request->telm) ? $request->telm : "0000000",
                    "country_code" => "EU",
                ]];
                $payment_body = array_merge($payment_body, $sdd_mandate);
            }
            $response = Http::withHeaders([
                'AccountId' => config('payment.accountId'),
                'ApiKey' => config('payment.apiKey'),
                'Content-Type' => 'application/json',
            ])->post('REMOVED API LINK', $payment_body);

            $this->addToPreOrdersTable($tkey);
            return $response->collect();
        }
    }
}

protected function addToPreOrdersTable(string $hash)
{
    $this->cartCollection();
    OrderService::createPreOrder($this->contents, $hash, 'buy');
}

addToPreOrdersTable Is also in both controllers.

 protected function index(Request $request)
    {
        if (isset($request->type)) {
            $result = $this->createRequest($request);
            $status = $result['status'];
            if ($status == 'ok') {
                $type = $result['method']['type'];
                switch ($type) {
                    case 'cc':
                        return redirect($result['method']['url']);
                    case 'mb':
                        return view('payment', ['type' => 'mb', 'data' => $result['method']]);
                    case 'mbw':
                        return view('payment', ['type' => 'mbw']);
                    case 'dd':
                        return view('payment', ['type' => 'dd', 'data' => $result['method']]);
                }
            }
            return back()->withInput()->withErrors(['Error' => 'An error has occured.']);
        }
        return view('payment');
    }
\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

Let's implements following: separate logic, refactoring of methods

Separate logic

Validate Requests/PaymentRequest.php add rule for validation

use Illuminate\Foundation\Http\FormRequest;

class PaymentRequest extends FormRequest
{
    public function rules(): array
    {
        return [
            'name' => ['required', 'string', 'in:cc,mb,mbw,dd'],
            'telm' => ['string'],
            'iban' =>  ['required', 'string']
        ];
    }
}

take out the logic and leave the thin controller:

Controllers/PaymentController

public function index(PaymentRequest $request, PaymentService $service) {

   return $service->run($request);
   
}

Now we are separating to service and external service. ExternalService - external request, service -our logic;

add Services/PaymentService.php and change method createRequest to send

    class PaymentService implements PaymentServiceInterface {

    private $externalPaymentService;
    public function __construct(ExternalPaymentService $externalPaymentService)
    {
        $this->externalPaymentService = $externalPaymentService;
    }

    public function run(PaymentRequest $request) {
        
        // Validate rule in PaymentRequest will not let you go further
        // if (isset($request->type)) { 
            $tkey = uniqid(Auth::id(), true);
            $result = $this->externalPaymentService->send($request, $tkey);
            $this->addToPreOrdersTable($tkey);
            $status = $result['status'];
            if ($status == 'ok') {
                $type = $result['method']['type'];
                switch ($type) {
                    case 'cc':
                        return redirect($result['method']['url']);
                    case 'mb':
                        return view('payment', ['type' => 'mb', 'data' => $result['method']]);
                    case 'mbw':
                        return view('payment', ['type' => 'mbw']);
                    case 'dd':
                        return view('payment', ['type' => 'dd', 'data' => $result['method']]);
                }
            }
            return back()->withInput()->withErrors(['Error' => 'An error has occured.']);
         //}
        // return view('payment');
    }

    protected function addToPreOrdersTable(string $hash)
    {
        $this->cartCollection();
        OrderService::createPreOrder($this->contents, $hash, 'buy');
    }

}

Services/PaymentServiceInterface.php

interface PaymentServiceInterface {
  run(Request $request); 
}

ExternalService/ExternalPaymentService.php

class ExternalPaymentService {
        public function send(Request $request, $tkey)
        {
            $this->cartCollection();

            $availableMethods = ['mb', 'mbw', 'cc', 'dd'];


            if (in_array($request->type, $availableMethods)) {
                $payment_body = [
                    "method" => $request->type,
                    "value" => (float)\Cart::getTotal(),
                    "currency" => "EUR",
                    "key" => $tkey,
                    "type" => "sale",
                    "customer" =>
                        [
                            "name" => Auth::user()->name,
                            "email" => Auth::user()->email,
                            "phone" => isset($request->telm) ? $request->telm : "",
                        ],
                    "capture" =>
                        [
                            "descriptive" => "Order",
                            "transaction_key" => $tkey,
                        ],
                ];
                if ($request->type == 'dd') {
                    $sdd_mandate =  ['sdd_mandate' => [
                        "iban" => $request->iban,
                        "account_holder" => Auth::user()->name,
                        "name" => Auth::user()->name,
                        "email" => Auth::user()->email,
                        "phone" => isset($request->telm) ? $request->telm : "0000000",
                        "country_code" => "EU",
                    ]];
                    $payment_body = array_merge($payment_body, $sdd_mandate);
                }
                $response = Http::withHeaders([
                    'AccountId' => config('payment.accountId'),
                    'ApiKey' => config('payment.apiKey'),
                    'Content-Type' => 'application/json',
                ])->post('REMOVED API LINK', $payment_body);


                return $response->collect();
            }
        }
    }

Okay, we made first level, separated logic!

Refactoring of methods

Let's create Services/PaymentsRenderStrategy.php and will changing run in PaymentService

Services/PaymentsRenderStrategy.php

class PaymentsRenderStrategy {
        private function cc($method) {
              return redirect($method['url']);
          }

          private function mb($method) {
              return view('payment', ['type' => 'mb', 'data' => $method]);
          }

          private function mbw($method) {
              return view('payment', ['type' => 'mbw']);
          }

          private function dd($method) {
              return view('payment', ['type' => 'dd', 'data' => $method]);
          }

          public function render($type, $data) {
              try {
               return $this->$type($data);
              } catch (Exception $e) {
                  throw new Exception('some error in methods');
              }
          }

    }

rewrite Services/PaymentService.php

    class PaymentService implements PaymentServiceInterface {

    private $externalPaymentService;

    public function __construct(ExternalPaymentService $externalPaymentService)
    {
        $this->externalPaymentService = $externalPaymentService;
    }

    public function run(PaymentRequest $request) {

        try {
            $tkey = uniqid(Auth::id(), true);
            $this->cartCollection();
            $result = $this->externalPaymentService->collect($request, $tkey)->send($request, $tkey);
            $this->addToPreOrdersTable($tkey);
            $status = $result['status'];

            if ($status != 'ok') {
                throw new Exception('Bad request');
            }
              return  (new PaymentsRenderStrategy())->render($request->type, ($result['method']);

        } catch (Exception $e) {
            return back()->withInput()->withErrors(['Error' => 'An error has occured.']);
        }
    }

    protected function addToPreOrdersTable(string $hash)
    {
        $this->cartCollection();
        OrderService::createPreOrder($this->contents, $hash, 'buy');
    }

}

Validateon we moved in PaymentRequest and we don't need extra checks like if (in_array($request->type, $availableMethods)).

$this->$type($data); - call need method by type (note: use $this->$type)

also we took the logic out of ExternalPaymentService and serparate method send by collect and send

ExternalServices/PaymentExternalService.php

class ExternalPaymentService {

       private $paymentBody;

       public function collect(PaymentRequest $request, $tkey) {
           $this->paymentBody = [
               "method" => $request->type,
               "value" => (float)\Cart::getTotal(),
               "currency" => "EUR",
               "key" => $tkey,
               "type" => "sale",
               "customer" =>
                   [
                       "name" => Auth::user()->name,
                       "email" => Auth::user()->email,
                       "phone" => isset($request->telm) ? $request->telm : "",
                   ],
               "capture" =>
                   [
                       "descriptive" => "Order",
                       "transaction_key" => $tkey,
                   ],
           ];
           if ($request->type == 'dd') {
               $sdd_mandate =  ['sdd_mandate' => [
                   "iban" => $request->iban,
                   "account_holder" => Auth::user()->name,
                   "name" => Auth::user()->name,
                   "email" => Auth::user()->email,
                   "phone" => isset($request->telm) ? $request->telm : "0000000",
                   "country_code" => "EU",
               ]];
               $this->paymentBody = array_merge($this->paymentBody, $sdd_mandate);
           }

           return $this;
       }

        public function send()
        {

                $response = Http::withHeaders([
                    'AccountId' => config('payment.accountId'),
                    'ApiKey' => config('payment.apiKey'),
                    'Content-Type' => 'application/json',
                ])->post('REMOVED API LINK', $this->paymentBody);


                return $response->collect();
            }
    }

Here we refactor I hope helped you. Next stage: take out magic constants (cc,mb,mbw,dd), to short methods ...etc. See more

\$\endgroup\$
2
\$\begingroup\$

There are two ways to reduce duplicate POST request code in Laravel/PHP for a payment system:

  • Use a trait. This allows you to define a reusable block of code that can be shared across multiple controllers.
  • Use a separate class. This allows you to encapsulate the common code in a single place, making it easier to maintain and update.

The approach you choose depends on your specific needs and preferences. If you need to reuse the code in a small number of controllers, a trait may be sufficient. If you need to reuse the code in a large number of controllers or if the code is complex, a separate class may be a better choice.

Here is an example of how to use a trait to reduce duplicate POST request code:

<?php

namespace App\Controllers\Traits;

use Illuminate\Http\Request;

trait PaymentRequestTrait
{
    protected function createRequest(Request $request)
    {
        $this->cartCollection();

        $availableMethods = ['mb', 'mbw', 'cc', 'dd'];
        $tkey = uniqid(Auth::id(), true);

        if (in_array($request->type, $availableMethods)) {
            $payment_body = [
                "method" => $request->type,
                "value" => (float)\Cart::getTotal(),
                "currency" => "EUR",
                "key" => $tkey,
                "type" => "sale",
                "customer" =>
                [
                    "name" => Auth::user()->name,
                    "email" => Auth::user()->email,
                    "phone" => isset($request->telm) ? $request->telm : "",
                ],
                "capture" =>
                [
                    "descriptive" => "Order",
                    "transaction_key" => $tkey,
                ],
            ];
            if ($request->type == 'dd') {
                $sdd_mandate =  ['sdd_mandate' => [
                    "iban" => $request->iban,
                    "account_holder" => Auth::user()->name,
                    "name" => Auth::user()->name,
                    "email" => Auth::user()->email,
                    "phone" => isset($request->telm) ? $request->telm : "0000000",
                    "country_code" => "EU",
                ]];
                $payment_body = array_merge($payment_body, $sdd_mandate);
            }
            $response = Http::withHeaders([
                'AccountId' => config('payment.accountId'),
                'ApiKey' => config('payment.apiKey'),
                'Content-Type' => 'application/json',
            ])->post('REMOVED API LINK', $payment_body);

            $this->addToPreOrdersTable($tkey);
            return $response->collect();
        }
    }

    protected function addToPreOrdersTable(string $hash)
    {
        $this->cartCollection();
        OrderService::createPreOrder($this->contents, $hash, 'buy');
    }
}

To use the trait, simply add it to the top of each controller that needs to create a payment request:

<?php

namespace App\Controllers;

use App\Controllers\Traits\PaymentRequestTrait;

class PaymentController extends Controller
{
    use PaymentRequestTrait;

    public function index(Request $request)
    {
        // ...
    }
}


<?php

namespace App\Controllers;

use App\Controllers\Traits\PaymentRequestTrait;

class PaymentExpressController extends Controller
{
    use PaymentRequestTrait;

    public function index(Request $request)
    {
        // ...
    }
}
\$\endgroup\$
1
\$\begingroup\$

I would create another service layer that would handle the creation of the request and maybe the call to the third party api. Your controllers are there to create the best response to the http request not really to create the best response, create external requests, handle them, etc..

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.