2
\$\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';
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 15, 2015 at 17:17
\$\endgroup\$

1 Answer 1

10
\$\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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Sep 20, 2015 at 16:15
\$\endgroup\$
2
  • \$\begingroup\$ Very useful information .I didn't know you could do that with request \$\endgroup\$ Commented Sep 23, 2015 at 17:49
  • \$\begingroup\$ Thanks for the feedback and good luck with your web app! \$\endgroup\$ Commented Sep 23, 2015 at 18:01

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.