I have 6 distinct images that a user needs to upload, so I have 6 different storeAs methods. When I have just one, everything works without fault, but 2 or more, I get a Call to a member function storeAs() on null error. Honesty, my code looks repetitive and dirty, so I'm not surprised.

public function createTentry($id)
{
    $trial = Trial::find($id);

    $image_plant_general = $this->image_plant_general->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_plant_general->getClientOriginalExtension(), 'trial-entry-photos');
    $image_plant_closeup = $this->image_plant_closeup->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_plant_closeup->getClientOriginalExtension(), 'trial-entry-photos');
    $image_fruit_in_plant = $this->image_fruit_in_plant->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_fruit_in_plant->getClientOriginalExtension(), 'trial-entry-photos');
    $image_fruit_in_plant_closeup = $this->image_fruit_in_plant_closeup->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_fruit_in_plant_closeup->getClientOriginalExtension(), 'trial-entry-photos');
    $image_fruit_in_harvest_single = $this->image_fruit_in_harvest_single->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_fruit_in_harvest_single->getClientOriginalExtension(), 'trial-entry-photos');
    $image_fruit_in_harvest_group = $this->image_fruit_in_harvest_group->storeAs('/', $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.'.$this->image_fruit_in_harvest_group->getClientOriginalExtension(), 'trial-entry-photos');

    Tentry::create([
        ...
        'image_plant_general' => $image_plant_general,
        'image_plant_closeup' => $image_plant_closeup,
        'image_fruit_in_plant' => $image_fruit_in_plant,
        'image_fruit_in_plant_closeup' => $image_fruit_in_plant_closeup,
        'image_fruit_in_harvest_single' => $image_fruit_in_harvest_single,
        'image_fruit_in_harvest_group' => $image_fruit_in_harvest_group,

    ]);

    return redirect()->route('trial.show', [$trial->evaluation_id, $trial->id]);
}

CodePudding user response：

I would recommend that you use an array and a loop, rather than checking individual properties. Makes for cleaner code, and easier to check the same thing over and over.

Another thing, I added model-route-binding, so that the Trial model is injected as a parameter directly without having to look it up explicitly.

public function createTentry(Trial $trial)
{
    $prefix = $this->trial->id.'_'.$this->id.'_'.now()->timestamp.'.';

    $images = [
        'image_plant_general',
        'image_plant_closeup',
        'image_fruit_in_plant',
        'image_fruit_in_plant_closeup',
        'image_fruit_in_harvest_single',
        'image_fruit_in_harvest_group'
    ];

    $data = [
        // Other Tentry-fields here
    ];

    foreach ($images as $image) {
        $data[$image] = $this->{$image}
            ? $this->{$image}->storeAs('/', $prefix.$this->image_plant_general->getClientOriginalExtension(), 'trial-entry-photos')
            : null;
    }

    Tentry::create($data);

    return redirect()->route('trial.show', [$trial->evaluation_id, $trial->id]);
}