Skip to main content
Code Review

Return to Answer

update wording, remove superfluous apostrophe https://english.stackexchange.com/a/8705/213844
Source Link

How does this look?

It looks okay. It appears to followmostly adhere to PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are appropriate.

Is there a cleaner way?

I’ve used Laravel for more than 8 years and grown to primarily use controller methods to update models. I've read about using the repository pattern to have thin controllers methods. I haven't seen the manual mention actions though I do see the laravel-actions package - is that what is being utilized here? 🤔

If you look at the source of the update() method within src/Illuminate/Database/Eloquent /Model.php notice that the last line of the method calls the fill() method before calling the save() method.

/**
 * Update the model in the database.
 *
 * @param array $attributes
 * @param array $options
 * @return bool
 */
public function update(array $attributes = [], array $options = [])
{
 if (! $this->exists) {
 return false;
 }
 return $this->fill($attributes)->save($options);
}

The fill() method calls $this->fillableFromArray() passing along the $attributes parameter.

/**
 * Fill the model with an array of attributes.
 *
 * @param array $attributes
 * @return $this
 *
 * @throws \Illuminate\Database\Eloquent\MassAssignmentException
 */
public function fill(array $attributes)
{
 $totallyGuarded = $this->totallyGuarded();
 $fillable = $this->fillableFromArray($attributes);

And that method calls getFillable() to determine which attributes can be filled based on the $attributes parameter.

/**
 * Get the fillable attributes of a given array.
 *
 * @param array $attributes
 * @return array
 */
protected function fillableFromArray(array $attributes)
{
 if (count($this->getFillable()) > 0 && ! static::$unguarded) {
 return array_intersect_key($attributes, array_flip($this->getFillable()));
 }
 return $attributes;
}

Consequently one can simply call the update() method passing in the attributes to be set on the model. The foreach loop can be removed and the save() call replaced with a call to update() - using a method to get the attributes from MyModelData or with a reflection class and it’sits getProperties() method.

How does this look?

It looks okay. It appears to follow PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are appropriate.

Is there a cleaner way?

I’ve used Laravel for more than 8 years and grown to primarily use controller methods to update models. I've read about using the repository pattern to have thin controllers methods. I haven't seen the manual mention actions though I do see the laravel-actions package - is that what is being utilized here? 🤔

If you look at the source of the update() method within src/Illuminate/Database/Eloquent /Model.php notice that the last line of the method calls the fill() method before calling the save() method.

/**
 * Update the model in the database.
 *
 * @param array $attributes
 * @param array $options
 * @return bool
 */
public function update(array $attributes = [], array $options = [])
{
 if (! $this->exists) {
 return false;
 }
 return $this->fill($attributes)->save($options);
}

The fill() method calls $this->fillableFromArray() passing along the $attributes parameter.

/**
 * Fill the model with an array of attributes.
 *
 * @param array $attributes
 * @return $this
 *
 * @throws \Illuminate\Database\Eloquent\MassAssignmentException
 */
public function fill(array $attributes)
{
 $totallyGuarded = $this->totallyGuarded();
 $fillable = $this->fillableFromArray($attributes);

And that method calls getFillable() to determine which attributes can be filled based on the $attributes parameter.

/**
 * Get the fillable attributes of a given array.
 *
 * @param array $attributes
 * @return array
 */
protected function fillableFromArray(array $attributes)
{
 if (count($this->getFillable()) > 0 && ! static::$unguarded) {
 return array_intersect_key($attributes, array_flip($this->getFillable()));
 }
 return $attributes;
}

Consequently one can simply call the update() method passing in the attributes to be set on the model. The foreach loop can be removed and the save() call replaced with a call to update() - using a method to get the attributes from MyModelData or with a reflection class and it’s getProperties() method.

How does this look?

It looks okay. It appears to mostly adhere to PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are appropriate.

Is there a cleaner way?

I’ve used Laravel for more than 8 years and grown to primarily use controller methods to update models. I've read about using the repository pattern to have thin controllers methods. I haven't seen the manual mention actions though I do see the laravel-actions package - is that what is being utilized here? 🤔

If you look at the source of the update() method within src/Illuminate/Database/Eloquent /Model.php notice that the last line of the method calls the fill() method before calling the save() method.

/**
 * Update the model in the database.
 *
 * @param array $attributes
 * @param array $options
 * @return bool
 */
public function update(array $attributes = [], array $options = [])
{
 if (! $this->exists) {
 return false;
 }
 return $this->fill($attributes)->save($options);
}

The fill() method calls $this->fillableFromArray() passing along the $attributes parameter.

/**
 * Fill the model with an array of attributes.
 *
 * @param array $attributes
 * @return $this
 *
 * @throws \Illuminate\Database\Eloquent\MassAssignmentException
 */
public function fill(array $attributes)
{
 $totallyGuarded = $this->totallyGuarded();
 $fillable = $this->fillableFromArray($attributes);

And that method calls getFillable() to determine which attributes can be filled based on the $attributes parameter.

/**
 * Get the fillable attributes of a given array.
 *
 * @param array $attributes
 * @return array
 */
protected function fillableFromArray(array $attributes)
{
 if (count($this->getFillable()) > 0 && ! static::$unguarded) {
 return array_intersect_key($attributes, array_flip($this->getFillable()));
 }
 return $attributes;
}

Consequently one can simply call the update() method passing in the attributes to be set on the model. The foreach loop can be removed and the save() call replaced with a call to update() - using a method to get the attributes from MyModelData or with a reflection class and its getProperties() method.

Source Link

How does this look?

It looks okay. It appears to follow PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are appropriate.

Is there a cleaner way?

I’ve used Laravel for more than 8 years and grown to primarily use controller methods to update models. I've read about using the repository pattern to have thin controllers methods. I haven't seen the manual mention actions though I do see the laravel-actions package - is that what is being utilized here? 🤔

If you look at the source of the update() method within src/Illuminate/Database/Eloquent /Model.php notice that the last line of the method calls the fill() method before calling the save() method.

/**
 * Update the model in the database.
 *
 * @param array $attributes
 * @param array $options
 * @return bool
 */
public function update(array $attributes = [], array $options = [])
{
 if (! $this->exists) {
 return false;
 }
 return $this->fill($attributes)->save($options);
}

The fill() method calls $this->fillableFromArray() passing along the $attributes parameter.

/**
 * Fill the model with an array of attributes.
 *
 * @param array $attributes
 * @return $this
 *
 * @throws \Illuminate\Database\Eloquent\MassAssignmentException
 */
public function fill(array $attributes)
{
 $totallyGuarded = $this->totallyGuarded();
 $fillable = $this->fillableFromArray($attributes);

And that method calls getFillable() to determine which attributes can be filled based on the $attributes parameter.

/**
 * Get the fillable attributes of a given array.
 *
 * @param array $attributes
 * @return array
 */
protected function fillableFromArray(array $attributes)
{
 if (count($this->getFillable()) > 0 && ! static::$unguarded) {
 return array_intersect_key($attributes, array_flip($this->getFillable()));
 }
 return $attributes;
}

Consequently one can simply call the update() method passing in the attributes to be set on the model. The foreach loop can be removed and the save() call replaced with a call to update() - using a method to get the attributes from MyModelData or with a reflection class and it’s getProperties() method.

lang-php

AltStyle によって変換されたページ (->オリジナル) /