Write function in controller below controller class?

Question

Is that a good practice to wrap some code from controller class method because it's too long and put that code in another custom function below controller class.

Here is controller method:

    public function store(UploadRequest $request)
{
    //Provjera duljine imena slike (max 20 znakova)
    if(!is_valid_name($request->file('file'))) {

        return redirect()->back()->withErrors(['File name can\'t be longer than 20 characters.']);
    }

    $user = Auth::user();
    $time = time();
    $image = $request->file('file');

    //Dodaj trenutno vrijeme prije imena slike kako bi se slika mogla identificirati
    $image_name = $time . $image->getClientOriginalName();

    //Ako je slika png spremi ju bez konverzije
    if($image->getClientOriginalExtension() == "png")
    {
        Storage::put('public/images/'.$user->id.'/png/'.$image_name, file_get_contents($image));
        save_image_to_database($image, $user, $time);
    }

Save_image_to_database() is my custom function that is written below controller class:

function save_image_to_database($image, $user, $time){

$db_image = new Image();

if($image->extension() == 'png')
{
    $db_image->path = $time . $image->getClientOriginalName();
    $db_image->png_size = $image->getClientOriginalSize();
}
else
{
    $path = $time . pathinfo($image->getClientOriginalName(), PATHINFO_FILENAME).'.png';
    $png_size = Storage::size('/public/images/'.$user->id.'/png/'.$path);

    $db_image->path = $path;
    $db_image->png_size = $png_size;
}

$db_image->user_id = $user->id;
$db_image->extension = $image->extension();
$db_image->size = $image->getClientSize();   
$db_image->save();

return redirect('/images');

}

Problem is that redirect() method in custom function is not working, it redirects to blank window but the path in browser "localhost:8000/images" is correct.When I manually refresh the site then it works and the view is returned. If I move that redirect() method from custom function to controller store() then it works well.

Should you maybe be returning the return of `save_image_to_database`? By that I mean use `return save_image_to_database($image, $user, $time);`. If your controller function doesn't return a `view()`, `redirect()`, `response()`, etc, you'll see a blank page when the function finishes. — Tim Lewis, Jul 19 '18 at 16:34
it redirects me to /images route that is in redirect(), but blank window appears, if I refresh it then the view appears — Denis2310, Jul 19 '18 at 17:01
Then whatever function is handling the `/images` route has some logic in it that is preventing a `return`; perhaps post the controller function for that? — Tim Lewis, Jul 19 '18 at 17:04
its weird that if I write that line of code "return redirect('/images');" below the call of save_image_to_database()" function in controller method then it works, but when it is in save_image_to_database() method then it wont work. — Denis2310, Jul 19 '18 at 17:06
Are you using `return save_image_to_database($image, $user, $time);` or not? I'm confused... — Tim Lewis, Jul 19 '18 at 17:10
no, I have just called that function like this: save_image_to_database($image, $user, $time); — Denis2310, Jul 19 '18 at 17:11
Try it with `return`, like I said in my first comment: *"By that I mean use `return save_image_to_database($image, $user, $time);`"* — Tim Lewis, Jul 19 '18 at 17:12
Have a read of the answer below; it basically says the same thing I did in these comments, so *"The most simple solution for you will be to return the response of helper function like `save_image_to_database($image, $user, $time);`"*, so consider upvoting/accepting their answer. — Tim Lewis, Jul 19 '18 at 17:16

Seva Kalashnikov · Answer 1 · 2018-07-19T17:18:05.080

Your function returns Response object but controller doesn't use it

The most simple solution for you will be to return the response of helper function like

return save_image_to_database($image, $user, $time);

However I would at least wrap it in try-catch to handle inability to save file like

try {
    save_image_to_database($image, $user, $time);
} catch (\Exception $e) {
    // return error response
}

// if we got here image was successfully saved
return redirect('/images');

Now back to your question about if its a good experience to handle image as another controller method.. I'd say no, you want to keep your controllers very skinny and have only methods that are used by Router

I'd either move it into a Trait (Traits in PHP) and use that trait in the controller or make an Image Service and use it via dependency injection.

Have a read of the comments above; also, consider adjusting your answer to `return save_image_to_database($image, $user, $time);` (you describe that, but then don't include it) — Tim Lewis, Jul 19 '18 at 17:17

Write function in controller below controller class?

1 Answers1