Duplicate Code: Make Controller Resourceful

The OrderController looks like a typical Resourceful Controller, but the author didn't go that route.

There are separate methods update() with PATCH and replace() with PUT:

routes/api_v1.php:

Route::apiResource('orders', OrderController::class)->except(['update']);
Route::patch('orders/{order}', [OrderController::class, 'update']);
Route::put('orders/{order}', [OrderController::class, 'replace']);

Looking at the Controller, those methods are IDENTICAL, except only for the Form Request classes: UpdateOrderRequest vs ReplaceOrderRequest:

app/Http/Controllers/Api/V1/OrderController.php:

use App\Http\Requests\Api\V1\ReplaceOrderRequest;
use App\Http\Requests\Api\V1\UpdateOrderRequest;
 
// ...
 
public function update(UpdateOrderRequest $request, $order_id)
{
    try {
        $order = Order::findOrFail($order_id);
        $this->isAble('update', $order); // policy
        $this->orderService->updateOrderHandleProducts($request, $order);
 
        return response()->json(new OrderResource($order), Response::HTTP_OK);
    } catch (ModelNotFoundException $eModelNotFound) {
        return $this->responseNotFound('Order not found');
    } catch (AuthorizationException $eAuthorizationException) {
        return $this->responseNotAuthorized();
    } catch (QueryException $eQueryException) {
        DB::rollback(); // Rollback transaction on database error
 
        return $this->responseDbError();
    } catch (Throwable $eTh) {
        DB::rollback(); // Rollback transaction on any other error
 
        return $this->responseUnexpectedError();
    }
}
 
public function replace(ReplaceOrderRequest $request, $order_id)
{
    // ... The same IDENTICAL 19 lines of code as update()
}

Digging deeper: looking at the Form Request classes, the ONLY difference is the required rules in the ReplaceOrderRequest vs. sometimes rules in the UpdateOrderRequest:

app/Http/Requests/Api/V1/UpdateOrderRequest.php:

return [
    'data' => 'required|array',
    'data.attributes' => 'sometimes|array',
    'data.relationships' => 'sometimes|array',

app/Http/Requests/Api/V1/ReplaceOrderRequest.php:

return [
    'data' => 'required|array',
    'data.attributes' => 'required|array',
    'data.relationships' => 'required|array',
 
    // ... ~10 more rules in the same fashion

To me, it just doesn't look/feel right. Too much duplicated code.

I don't know the reasons behind why it was created this way, but it doesn't "feel" like Laravel. A single update() method should handle both cases.

I tried to understand the reasoning from the real-life scenario angle:

• There's an endpoint to REPLACE the order with some other data which is required
• Another endpoint to UPDATE the order, and then details may be EMPTY?

It just doesn't make much sense to me, but maybe I'm missing something here.

So, here's what I did:

• UpdateOrderRequest: replaced sometimes with required
• Deleted ReplaceOrderRequest file
• Deleted replace() method in Controller
• The final desired result: transformed the Controller into a proper CRUD format, with Route::apiResource() only

routes/api_v1.php:

Route::apiResource('orders', OrderController::class)->except(['update']); 
Route::patch('orders/{order}', [OrderController::class, 'update']); 
Route::put('orders/{order}', [OrderController::class, 'replace']); 
Route::apiResource('orders', OrderController::class); 

Here's the full GitHub commit for that.

Warning: This was a RISKY Refactoring

I'm not 100% sure about this refactoring and whether I didn't break anything. Executed php artisan test: no errors.

But the actual point of this lesson is this: if someone does something non-standard, it's hard to understand their reasons and refactor code. So, don't "fight the framework" and stick to the standard ways of structuring things.