User Avatar upload

Question

I have been looking at some coding videos on Laracasts and the focus seems to be very readable code and keeping the controller clean. How can I clean this up? This code takes an uploaded user avatar, stores the avatar image on the file system and stores the name in a database.

public function update(Request $request, $id)
{
    $this->validate($request, [
    'avatar' => 'required|image|max:10000|mimes:jpg,jpeg,gif,bmp,png'
    ]);

    $UploadedFile = $request->file('avatar');

    $name = renameFile($UploadedFile); // helper function.

    $user = User::where('id', '=', $id)->first();

    if(!is_null($user->avatar)){ // avatar's name stored in user table

        Storage::disk('public')->delete('avatars/' . $user->avatar);
    };

    $image = Image::make($UploadedFile)
        ->resize(400, null, function ($constraint) { $constraint->aspectRatio(); } )
        ->encode('jpg', 80);

    Storage::disk('public')->put('avatars/' . $name, $image);

    $user->avatar = $name;

    $user->save();

    return back()->with(message('User Profile Photo has been updated!', 'success'));

};

It works fine. It's just the presentation that's bothering me.

I have looked into repositories, interfaces etc but can't seem to figure out when to use what.

Answer 1

I honestly don't think there is much to "clean up" here, as this seems pretty straight forward. A few stylistic notes:

• It is pretty standard in PHP to have variables named with first letter in lower case. So consider $UploadedFile => $uploadedFile.
• Why blank line between most every line of code? This seems excessive. Vertical white space can be good to group various logical sections of code, but in your cases it seems to be overkill.
• Don't put comments at the end of lines. Put them on the line before the code to which they relate. In this case, I don;t think you even need to the comments you have.

---

From a logic standpoint, I don't think it makes sense for you to delete the existing avatar file until after you sucessfully format and write the new one. What happens if something fails during the process of transforming and writing the new image file? You have now destroyed the existing file without a valid replacement.

---

Your code is very happy-path oriented. If something fails, it is unclear how you message the user regarding that failure.