Как перенести код из контроллера в модель?

Question

[acv56]

Здравствуйте. Есть контроллер на Yii2:

public function actionUpdate($id)
{
	$model 	= $this->findModel($id);
	$image 	= $model->image;	

	if ($model->load(Yii::$app->request->post())) {
		$model->files 			= UploadedFile::getInstance($model, 'image');
			
		if ($model->validate()) {
		
			if ($model->files) {
				$name 			= $model->files->baseName . '.' . $model->files->extension;
				$path			= Yii::getAlias('@webroot' . '/uploads/') . $name;
				$model->files->saveAs($path);
				$model->image 	= $name;
					
				if($model->removeImage) {
					$fileRemove 	= Yii::getAlias('@webroot' . '/uploads/') . $image;
					if(file_exists($fileRemove)) {
						unlink($fileRemove);
						$model->image 	= '';
					}
				}
			}

			$model->save();
			return $this->redirect(['view', 'id' => $model->id]);		
		}
		
		
	} 
	
	else {
		return $this->render('update', [
			'model' 			=> $model,
		]);
	}

}

В модели определил свойства:

public $files;
public $removeImage;

Там еще есть rules, и т.п.
Все работает, но у меня все контроллеры получаются толстые. Я хочу перенести код в модель, чтобы сделать тонкие контроллеры. Какие участки кода туда можно перенести и как вызвать в контроллере?
Нужно ли туда переносить из контроллера такое? :

Article::findOne($id)
Article::find()->where(['id' => $id])->all();

Answer 1

[SharuPoNemnogu]

public function actionUpdate($id)
{
    $model = $this->findModel($id);
    if ($model->load(Yii::$app->request->post()) && $model->validate()) {
        $model->save();
        return $this->redirect(['view', 'id' => $model->id]);
    } else {
        return $this->render('update', ['model' => $model]);
    }
}

вот это оставить, все остальное в модель в beforeSave()

Comments

[acv56]

а beforeSave() не нужно в экшине прописывать?

[SharuPoNemnogu]

нет, он автоматом вызывается, в доке посмотрите

[Никита]

beforeSave() зло. В нем нельзя писать логику, если не хочется отгрести в будущем. Лучше создать отдельный метод для конкретного случая.

[SharuPoNemnogu]

Никита: поясните в чем зло

[acv56]

SharuPoNemnogu: спасибо. А как быть например с $model->removeImage? вместе $model что прописывать? $this?

[acv56]

Никита: почему зло? beforeSave() плохо?

[SharuPoNemnogu]

acv56: да

[acv56]

SharuPoNemnogu: переместил в модель, но не работает вот этот участок, if($this->removeImage) он не передает нажат ли чекбокс и не перезаписывает $this->image = ''; как будто он не сохраняет все что в модели внутри beforeSave

[SharuPoNemnogu]

acv56: покажите код

[acv56]

SharuPoNemnogu:

public function beforeSave($insert)
	{
		if (parent::beforeSave($insert)) {

		$this->files 			= UploadedFile::getInstance($this, 'image');

		if ($this->validate()) {

		if ($this->files) {
		$name 			= $this->files->baseName . '.' . $this->files->extension;
		$path			= Yii::getAlias('@webroot' . '/uploads/') . $name;
		$this->files->saveAs($path);
		$this->image 	= $name;
		  
		if($this->removeImage) {
		  $fileRemove 	= Yii::getAlias('@webroot' . '/uploads/') . $image;
		  if(file_exists($fileRemove)) {
			unlink($fileRemove);
			$this->image 	= '';
		  }
		}
		}
	  
	  
		return true;
	} else {
		return false;
	}
}

[acv56]

SharuPoNemnogu: разобрался, работает, спасибо огромное!

[Никита]

SharuPoNemnogu: >"поясните в чем зло"
В том, что этот метод пытаются часто засунуть то, чего там не должно быть. ( Я сам нарывался на это часто и знаю последствия ) А не должно быть там бизнес-логики.

Приведу пример. Ваша модель может отвечать не за одну бизнес-логику, а допустим хотяб две. Что в таком случае будет с кодом? Несколько безобидных условий? А если взять 3 разных процесса. Там такая каша начнётся, что уже через 3 дня с трудом будете понимать что происходит. А теперь приходит новенький и у него задание что-то поправить в этой логике: он не сразу найдёт что происходит (евент вызывается ведь неявно), после он догадается что всё в одном методе лежит и без особых усилий просто не прочтёт что там внутри.

Методы (евенты, как угодно) beforeSave() и afterSave() очень хороши для данных, которые всегда должны идти параллельно тем, за какие данные отвечает модель. Например, загрузка картинки параллельно записи в БД очень неплохо смотрится в этих евентах. Но автора явно интересует вопрос не только по картинкам.

[acv56]

Никита: тогда что вы предлагаете? может просто в моделе функцию и вызывать ее из контроллера?

[SharuPoNemnogu]

Никита: напихать бизнес-логики можно куда угодно. Мы в контексте данной задачи сейчас рассматриваем, я не говорил, что все нужно пихать в beforeSave. Если вынести это в отдельный метод, то тот же новенький как вы говорите, просто не будет знать, что нужно загрузить картинку и не вызовет этот метод. С таким подходом можно сказать, что и триггеры в бд это зло, они же вызываются не явно.
Я в чем то согласен с вашей точкой зрения, но не в контексте данной задачи.

Answer 2

[Никита]

Перекинули внешние данные в модель, взяли данные из неё и передали в представление. Это все займет 4-5 строк.