0

In my project, I have duplicated functions in my Controllers as each Controller needs to have similar functionality but it feels slightly dirty to repeat code.

In both my EventController and my ArticleController I have a duplicated function called handleTags() that literally does the same thing in each model.

The code looks like this:

/**
* Handle tagging of this resource
*
* First, get the tags field and make sure it isn't empty
* Convert the string into an array and loop through the array
* Create new tags if they don't exist
*
* @param Request $request: data from the request
* @param int $id: the ID of the model instance have tags synced to
*/
public function handleTags(Request $request, $id)
{
    $event = Event::find($id);

    if ($request->has('tags')) {
        $tags = $request->get('tags');

        if (!empty($tags)) {
            // Turn a String into an array E.g. one, two
            $tagArray = array_filter(explode(", ", $tags));

            // Loop through the tag array that we just created
            foreach ($tagArray as $tag) {
                Tag::firstOrCreate([
                        'name' => ucfirst(trim($tag)),
                        'slug' => str_slug($tag)
                    ]);
            }

            // Grab the IDs for the tags in the array
            $tags = Tag::whereIn('name', $tagArray)->get()->pluck('id');
            $event->tags()->sync($tags);
        } else {
            // If there were no tags, remove them from this model instance
            $event->tags()->sync(array());
        }
    }
}

Would it be possible to move this functionality into a Trait? Something like Taggable?

Then you would call handleTags() in the relevant Controllers via the trait, in the same way Searchable gives you access to the search() method?

3
  • 2
    sure, php wont stop you
    – lagbox
    Aug 17, 2018 at 9:32
  • Laravel itself already decorates its boilerplate controller with various traits like use AuthorizesRequests, DispatchesJobs, ValidatesRequests; so nothing wrong with using traits
    – apokryfos
    Aug 17, 2018 at 9:34
  • You won't even need to create another function in the controller to wrap this function. You can simply use this method in your routes directly (only if desired of course).
    – Namoshek
    Aug 17, 2018 at 9:35

2 Answers 2

1

I think that a better solution will be to make a Model trait, I will explain my self.

trait HasTags {

    public function handleTags($tags) 
    {
        $tags = array_filter(explode(", ", $tags))

        $tags = array_map(function () {
            return Tag::firstOrCreate([
                'name' => ucfirst(trim($tag)),
                'slug' => str_slug($tag)
            ]);
        }, $tags)

        $this->tags()->sync(collect($tags)->pluck('id'))
    }

    public function tags()
    {
        return $this->morphMany(Tag::class);
    }

}

Model

class Event extends Model
{
    use HasTags;
}

Controller

$event = Event::find($id);

if ($request->has('tags')) {
    $event->handleTags($request->get('tags'));
}

I Write it very quickly and without testing it but this is the general idea. you can event have more refactoring by using all the array manipulations with collections.

1

You can make a trait in app/Http/Traits/TaggableTrait.php

You just need to pass an object instead of the id, so that the function will be independent from the class type.

then your trait will be something like this :

namespace App\Http\Traits;

use App\Tag;

trait TaggableTrait
{
    /**
    * @param Request $request: data from the request
    * @param App\Article | App\Event $object: the model instance have tags synced to
    */
    public function handleTags(Request $request, $object)
    {
        if ($request->has('tags')) {
            $tags = $request->get('tags');

            if (!empty($tags)) {
                  // Turn a String into an array E.g. one, two
                  $tagArray = array_filter(explode(", ", $tags));

                  // Loop through the tag array that we just created
                  foreach ($tagArray as $tag) {
                      Tag::firstOrCreate([
                         'name' => ucfirst(trim($tag)),
                         'slug' => str_slug($tag)
                      ]);
                  }

                  // Grab the IDs for the tags in the array
                  $tags = Tag::whereIn('name', $tagArray)->get()->pluck('id');
                  $object->tags()->sync($tags);
            } else {
                // If there were no tags, remove them from this model instance
                $object->tags()->sync(array());
            }
        }
    }
}

EventController

use App\Http\Traits\TaggableTrait;

class EventController extends Controller
{
    use TaggableTrait;

    /*** ***** */

    public function update(Request $request, $id)
    {
        /** ***/

        $event = Event::findOrFail($id);
        handleTags($request, $event);

       /*** *** */
    }       
}
2
  • Is it okay to have Traits on a Controller? Aug 17, 2018 at 11:13
  • yeah, this the way we use it Laravel, check out LoginController
    – bondif
    Aug 17, 2018 at 14:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.