Laravel controller to update appointment requests

Question

My question/refactor is similar to what is found here: Laravel controller for form validation skinny

However, I feel that the answer is for 5.0 and not 5.2. This question does not pertain only to one controller but considering that I am making a RESTful API I believe 1 example is sufficient so I can propagate this:

class UpdateAppointmentRequest extends Request
{
 /**
 * Determine if the user is authorized to make this request.
 *
 * @return bool
 */
 public function authorize()
 {
 return false;
 }
 /**
 * Get the validation rules that apply to the request.
 *
 * @return array
 */
 public function rules()
 {
 return [
 'id' => 'required|integer',
 'user_id' => 'required|integer',
 'status' => 'integer',
 'note' => 'alpha'
 ];
 }
}
public function update(UpdateAppointmentRequest $request)
{
 $appointment = $this->appointment->find($request->get('id'));
 if ($appointment === null) {
 return $this->notFound('Appointment Not Found');
 }
 if($request->has('user_id') === true) {
 $appointment->user_id = $request->get('user_id');
 }
 if($request->has('status') === true) {
 $appointment->status = $request->get('status');
 } 
 if($request->has('note') === true) {
 $appointment->note = $request->get('note');
 } 
 try {
 return response()->json(['created' => $appointment->save()]);
 } catch(\PDOException $e) {
 return $this->internalIssues($e->getMessage());
 }
}

My issue is the if statements. Now considering that only 2 items from the validation are required and the rest are not - I have to validate if the present of the variable is there in order to update. It works well, but it will eventually make controllers very bloated (depending on which restful resources might have more parameters).

Does anyone know of a better way to refactor this within Laravel 5.2+?

• jeremy: could always use ternaries just to make it look cleaner
• azngunit81: @Jeremy dont like using ternaries - it just makes it look cleaner but doesnt make it better.
• Marco Aurélio Deleu: is $this->appointmentan Eloquent Model?
• azngunit81: @MarcoAurélioDeleu yesm injected from constructor

Answer 1

public function update(UpdateAppointmentRequest $request)
{
 try {
 $data = array_filter($request->only(['user_id','status','note']));
 $appointment = $this->appointment->findOrFail($request->input('id'));
 $result = $appointment->update($data);
 return response()->json(['created' => $result]);
 } catch(\PDOException $e) {
 return $this->internalIssues($e->getMessage());
 }
}

What is wrong with this simple solution? If request does not have a field, say user_id, the only would not return that. And if there is an error while updating, the catch would catch that and return error. Why all those manual checking?

• Jamal: Is this a separate question, or is it a review of the OP's code? It's a little hard to tell.
• vfsoraki: I just just thought that OP did not use findOrFail and update purposefully, so that made my tone. Sorry if it is a bit question-y, but let OP give his/her opinion.
• azngunit81: your findOrFail approach is good. that escaped me, it will take care of the check in one line. However your ONLY solution is wrong. because only takes a subset of the request. IF status is absent then you will be updating it with null values and overriding previous values.
• vfsoraki: @azngunit81 Look at it now. You were right, I thought it would filter it by itself, but it did not. using array_filter would solve that issue.