Refactoring my store() method

I would add a mutator attribute for the image, in which you can manage the image, and store it's path. Something like this:

// in your Post model

public function setImageAttribute( $file )
{
    if ($file)
    {
        // perform the store here
        
        // save the file path to the database field
        $this->attributes['image'] = $fileNameToStore;
    } 
}

Then you can use just this:

Post::create([
    'title' => request('title'),
    'description' => request('description'),
    'prize' => request('prize'),
    'location' => request('location'),
    'image' => request('image')
]);

Also depends on the content of request but you could use Post::create($request->all()), or even Post::create($request->only(['title','description','prize','location','image']))

Whats wrong with what you showed?

Does the mutator look like this?

public function setImageAttribute($image)
{

    if($request->hasFile('image'))
    {
        $fileNameExtension = $request->file('image')->getClientOriginalName();
        $fileName = pathinfo($fileNameExtension, PATHINFO_FILENAME);
        $extension = $request->file('image')->getClientOriginalExtension();
        $fileNameToStore = $fileName.'_'.time().'.'.$extension;
        $path = $request->file('image')->storeAs('public/posts_image', $fileNameToStore);
    }  
    else 
    {
        $fileNameToStore = 'default.jpg';
    }

}

Nothing is wrong with the commented code, its just I want to learn more how on refactoring cleaner codes :D

Previously the way I store my image was like this

   if($request->hasFile('image'))
    {
        $fileNameExtension = $request->file('image')->getClientOriginalName();
        $fileName = pathinfo($fileNameExtension, PATHINFO_FILENAME);
        $extension = $request->file('image')->getClientOriginalExtension();
        $fileNameToStore = $fileName.'_'.time().'.'.$extension;
        $path = $request->file('image')->storeAs('public/posts_image', $fileNameToStore);
    }  
    else 
    {
        $fileNameToStore = 'default.jpg';
    }

$post = new Post();
    $post->title = $request->input('title');
    $post->description = $request->input('description');
    $post->prize = $request->input('prize');
    $post->location = $request->input('location');
    $post->image = $fileNameToStore;
    $post->save();

You can't get away from your image code (although it contains mistakes)

You could use create like

$post = Post::create(
        $request->only(['title', 'description', 'prize', 'location']) + 
        ['image'=>$fileNameToStore]
    );

@honeybear the mutator looks good, just don't forget to add the new value to the attributes array at the bottom of your method:

$this->attributes['image'] = $fileNameToStore;

And also you cannot use $request within the method because you are not passing it, just use the $image as that's already the file.

So your if statement will be

if($image) ...

Alright, I'll try. First time I'm using the mutator method.

I updated the mutator like shown below, if I uploaded a post without image it is success. However if theres an image, it is not storing the uploaded image and post info into the storage link or database. Am I missing something?

(mutator)

 public function setImageAttribute($image)
{

    if($image)
    {
        $fileNameExtension = $request->file('image')->getClientOriginalName();
        $fileName = pathinfo($fileNameExtension, PATHINFO_FILENAME);
        $extension = $request->file('image')->getClientOriginalExtension();
        $fileNameToStore = $fileName.'_'.time().'.'.$extension;
        $path = $request->file('image')->storeAs('public/posts_image', $fileNameToStore);
    }  
    else 
    {
        $fileNameToStore = 'default.jpg';
    }
    $this->attributes['image'] = $fileNameToStore;
}

(postscontroller)

 public function store(Request $request)
{
$this->validate($request,[
        'title' => 'required',
        'description' => 'required',
        'prize' => 'required',
        'location' => 'required',
        'image' => 'image|max:1999'
    ]);

    Post::create([
        'title' => request('title'),
        'description' => request('description'),
        'prize' => request('prize'),
        'location' => request('location'),
        'image' => request('image')
    ]);
    
    return redirect('/posts')->with('success', 'Post Created');
    
}

May I know what mistakes have I done, would like to improve my coding approach.

@honeybear you should read my answer above carefully. I said that you cannot use $request within your mutator, it should thrown an exception using $request when it does not exists.

$image is already $request->file('image'); so replace it like so:

$fileNameExtension = $image->getClientOriginalName();
$fileName = pathinfo($fileNameExtension, PATHINFO_FILENAME);
$extension = $image->getClientOriginalExtension();
$fileNameToStore = $fileName.'_'.time().'.'.$extension;
$path = $image->storeAs('public/posts_image', $fileNameToStore);

However, the approach you showed me did work. Thank you :)

@honeybear you should accept the answer that worked for you :) the best way to thank someone that takes time to help :)

Ohh, my mistake. I didnt notice theres a request inside the storing method.. its late tho haha. Thank you.. tried it and it works. Thanks again nakov, you've help me so much throughout my learning process :)

Your mutator approach seems cleaner for my controller, thank you so much again :)