1
\$\begingroup\$

I am building my first Laravel web app and I have created a class to handle API requests for a model called Quote. In what ways can the code be improved or reduced?

    <?php
namespace App\Http\Controllers;

use Illuminate\Http;
use Illuminate\Http\Request;
use App\SuperQuote\Transformers\QuoteTransformer;
use App\SuperQuote\Transformers\CustomValidation;
use App\SuperQuote\ClassHelpers\ArrayHelper;
use App\SuperQuote\Repositories\RepositoryInterface;
use Illuminate\Support\Facades\Input;
use Illuminate\Support\Facades\Validator;

class AppQuoteDatabaseController extends ApiController
{

    /**
     * @var  App\SuperQuote\Transformers\QuoteTransformer
     *
     */
    protected $quoteTransformer;

    /**
     * @var  App\SuperQuote\Transformers\CustomValidation
     *
     */
    protected $customValidation;

    /**
     * @var  App\SuperQuote\Classhelper\ArrayHelper
     *
     */
    protected $arrayHelper;

    /**
     * @var  app\SuperQuote\Repositories\RepositoryInterface;
     *
     */
    protected $appQuoteDatabase;


    function __construct(QuoteTransformer $quoteTransformer, CustomValidation $customValidation,ArrayHelper $arrayHelper,RepositoryInterface $appQuoteDatabase)
    {
        $this->quoteTransformer = $quoteTransformer;
        $this->customValidation = $customValidation;
        $this->arrayHelper = $arrayHelper;
        $this->appQuoteDatabase = $appQuoteDatabase;
    }


    /**
     * @return Http\JsonResponse
     */
    public function index()
    {
        $quotes=$this->appQuoteDatabase->getAll();

        if ($quotes != null) {
            return $this->successfullRequest($this->quoteTransformer->transformCollection($quotes->toArray()));
        } else {
            return $this->internalError();
        }
    }


    /**
     * @param $name
     * @return Http\JsonResponse
     */
    public function GetQuoteByName($name)
    {

        $quotes=$this->appQuoteDatabase->findbyName($name);
        if (count($quotes) > 0)
        {
            return $this->successfullRequest($this->quoteTransformer->transformCollection($quotes->toArray()));
        }
        else
        {
            return $this->respondNotFound('database does not contain that author, You can always add their information to the database');
        }


    }

    /**
     * @return Http\JsonResponse
     */
    public function edit()
    {

        if (array_key_exists('id', $_GET) &&( array_key_exists('quote', $_GET) || array_key_exists('authorName', $_GET) || array_key_exists('category', $_GET)  ))
        {
            $quote=$this->appQuoteDatabase->find($_GET['id']);

            if($quote != null)
            {
                $this->arrayHelper->updateField($quote,'category',$this->customValidation);
                $this->arrayHelper->updateField($quote,'author',$this->customValidation);
                $this->arrayHelper->updateField($quote,'quote',$this->customValidation);
                $this->arrayHelper->updateField($quote,'quoteOrigin',$this->customValidation);
                $this->arrayHelper->updateField($quote,'dateOrigin',$this->customValidation);

                $quote->save();

                return $this->successfullRequest($this->quoteTransformer->transform($quote));
            }
        }

        return $this->respondNotFound('Check values that were passed');
    }

    /**
     * @return Http\JsonResponse
     * @internal param Request $request
     * @internal param $date
     */
    public function findQuotesBetweenDate()
    {
        $validator = Validator::make(
            array(
                'startDate' => Input::get( 'startDate' ),
                'endDate' => Input::get( 'endDate' )

            ),
            array(
                'startDate' => array( 'required','date' ),
                'endDate' => array( 'required','date' ),

            )
        );
        if ($validator->fails())
        {

            $startDateCheck = $this->customValidation->IsNullOrEmptyString($_GET['startDate']);
            $endDateCheck = $this->customValidation->IsNullOrEmptyString($_GET['endDate']);

            if ($endDateCheck === false && $startDateCheck === false)
            {
              $quotes=$this->appQuoteDatabase->findbyDates($_GET['startDate'], $_GET['endDate'] );
                if (count($quotes) > 0)
                {
                    return $this->successfullRequest($this->quoteTransformer->transformCollection($quotes->toArray()));
                }
                else
                {
                    return $this->respondNotFound('database does not contain information related to that date');
                }

            }

        }
        return $this->respondNotFound($validator->messages());
    }

    /**
     * @param Request $request
     * @return Http\JsonResponse
     */
    public function store(Request $request)
    {

        $validator = Validator::make(
            array(
                'authorName' => Input::get( 'authorName' ),
                'quote' => Input::get( 'quote' ),
                'category' => Input::get( 'category' )
            ),
            array(
                'name' => array( 'required' ),
                'quote' => array( 'required' ),
                'category' => array( 'required' )
            )
        );

        if ($validator->fails())
        {
            $authorNameValidation = $this->customValidation->IsNullOrEmptyString($request->input('authorName'));
            $quoteValidation = $this->customValidation->IsNullOrEmptyString($request->input('quote'));
            $categoryValidation = $this->customValidation->IsNullOrEmptyString($request->input('category'));

            if ($authorNameValidation === false && $quoteValidation === false && $categoryValidation === false) {
                $values = array();
                $values['authorName'] = $request->input('authorName');
                $values['quote']= $request->input('quote');
                $values['category'] = $request->input('category');

                $values->save();

                if ($this->appQuoteDatabase->insert($values) > 0) {
                    return $this->successfullRequestWithMessage(true);
                } else {
                    return $this->internalError();
                }

            }

        }
        return $this->respondNotFound($validator->messages());

    }


    /**
     * @param $value
     * @return response()->json
     **/
    public function show($value)
    {
        $itemFound=$this->appQuoteDatabase->find($value);

        if ($itemFound != null) {
            return $this->successfullRequest($this->quoteTransformer->transform($itemFound));
        } else {

            return $this->respondNotFound('Quote Not Found');
        }

    }



}



  <?php namespace App\Model;

use Illuminate\Database\Eloquent\Model;

    class AppQuoteDatabase extends Model {

    protected $table = 'app_quote_database';

}
\$\endgroup\$
9
\$\begingroup\$

Since you are using Laravel, I would suggest to use some of the cool features of Laravel:

  1. Not necessary, but maybe you want to use the Laravel request feature instead of the $_GET variable:

    $request->input('id')
    

    instead of:

    $_GET['id']
    
  2. Laravel input validation is more powerful than you think. Instead of the following if-statement:

    if (array_key_exists('id', $_GET) &&( array_key_exists('quote', $_GET) || array_key_exists('authorName', $_GET) || array_key_exists('category', $_GET)  ))
    {
        ...
    }
    return $this->respondNotFound('Check values that were passed');
    

    you might want to use the Laravel validation:

    $validator = Validator::make($request->all(), [
        'id' => 'required',
        'quote' => 'required_without_all:authorName,category',
        'authorName' => 'required_without_all:quote,category',
        'category' => 'required_without_all:quote,authorName',
    ]);
    if ($validator->fails()) {
        return $this->respondNotFound('Check values that were passed');
    }
    ...
    

    The advantage is that you can add additional and more complex checks easily, i.e. you can use 'id' => 'required|numeric' to validate whether id is a number.

  3. Again, Laravel can do more than you might expect. Replace:

    $validator = Validator::make(
        array(
            'startDate' => Input::get( 'startDate' ),
            'endDate' => Input::get( 'endDate' )
        ),
        array(
            'startDate' => array( 'required','date' ),
            'endDate' => array( 'required','date' ),
        )
    );
    

    with:

    $validator = Validator::make($request->all(),
        array(
            'startDate' => array( 'required','date' ),
            'endDate' => array( 'required','date' ),
        )
    );
    

    Laravel will automatically compare the startDate and endDate from $_GET or $_POST with your validation requirements. You only need to specify the requirements, building an input array is usually not necessary. And the same principle can be applied for the other validation.

  4. In Laravel it is possible to get certain values from $request, so replacing:

    $values = array();
    $values['authorName'] = $request->input('authorName');
    $values['quote']= $request->input('quote');
    $values['category'] = $request->input('category');
    

    with:

    $values = $request->only(['authorName', 'quote', 'category']);
    

    should do exactly the same, but looks prettier.

I have not validated my code, so if you face any problems with it, let me know.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Very useful information .I didn't know you could do that with request \$\endgroup\$ – DacorieS Sep 23 '15 at 17:49
  • \$\begingroup\$ Thanks for the feedback and good luck with your web app! \$\endgroup\$ – Xeros Sep 23 '15 at 18:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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