Блог

Я хочу, чтобы, следуя правилу, одна функция выполняла только одну вещь. Поэтому мне нужно переработать этот метод. Которые делают много вещей для выполнения задачи (настройки). В основном внутри каждого кода код будет повторно использоваться для другого метода (setGenres, setLanguage, setCharacter, setCountries) с тем же кодом стиля.

Это сырой оригинальный метод. в нижней части моего метода рефакторинга.

 public function setTags($tags)
{
    foreach ($tags as $name)
    {
        $tag = Tag::firstOrCreate([
            'name' => ucwords($name)
        ]);

        if (empty($this->media->tags()->where('tag_id', $tag->id)->exists())) {
            $this->media->tags()->attach($tag->id);
        }
    }
}
 

Способ рефакторинга

 // is it right to name it createModelValue?
public function createModelValue($model, $value) {
  $collection = $model::firstOrCreate([
                  'name' => ucwords($value)
                ]);

  if (!empty($collection)) {
    return $collection;
  }else{ return NULL; }
}

// new problem how to pass $model (tag) as function relationship to replace tags function
public function setModel($model) {
  if (empty($this->media->tags()->where("{$model}_id", $model->id)->exists())) {
              $this->media->tags()->attach($model->id);
  }
}
 

Laravel есть sync() метод, который пригодится здесь, вместо того, чтобы прикреплять каждый тег по отдельности, вы можете просто сказать прикрепить все теги со следующим идентификатором и удалить все остальное.

 $tagsCreated = collect($tags)->map(function (string $tag) {
    return Tag::firstOrCreate([
        'name' => ucwords($tag)
    ]);
});

$this->media->tags()->sync($tagsCreated);
 

Честно говоря, быть откровенным в коде иногда лучше, чем быть умным. Ваша первая версия вашего метода очень конкретна и четко указывает, что она делает. Твоя переработанная умная версия, я подумал: «э-э, что происходит». Поэтому, выбирая между этими двумя, я хотел бы выбрать то, что указано выше. Кроме того, ваш метод выполняет только одно, он устанавливает теги из массива строк. Можно ли это оптимизировать, да, но очень незначительно. Этот принцип более применим, если у вас есть служба электронной почты, которая отправляет электронные письма, вы не должны начинать создавать PDF-файлы одновременно.

Я сделал аналогичную логику, как и в рефакторинге, но подумайте, насколько велик проект и сколько раз вы повторяли эту логику, прежде чем начать это.

Универсальный рефакторинг может быть таким же простым, как этот, если вы чувствуете себя предприимчивым, вы можете реализовать теги в качестве признака и/или интерфейса.

 function setTags($model, $tags) {
    $tagsCreated = collect($tags)->map(function (string $tag) {
        return Tag::firstOrCreate([
            'name' => ucwords($tag)
        ]);
    });

    $model->tags()->sync($tagsCreated);
}