forceToggle (or better name) for attaching/detaching many-to-many relationships

[glamorous]

Hi,

Recently I had to write this piece of code:

if ($request->boolean('is_favorite')) {
    $currentUser->favoriteArticles()->syncWithoutDetaching([$article->id]);
} else {
    $currentUser->favoriteArticles()->detach($article->id);
}

Simply explained: when endpoint is given "is_favorite" true, it should attach the relation (but not giving duplicate errors, so we use syncWithoutDetaching). When "false" is givin, we should remove the relation (which gives no errors)

The idea was to provide of forceToggle($ids, $boolean) function to the InteractsWithPivotTable trait. The code could than be a lot cleaner to:

$currentUser->favoriteArticles()->forceToggle($article->id, $request->boolean('is_favorite');

Should I make a PR, is this helpfull for others too? And any suggestion to maybe even a better, more clear name of the function/ability?

[danielgestwa]

I would say that passing true/false flags to functions changing it's behavior is generally not a good practice.
You should keep using two different functions addToFavourites(), removeFromFavourites() which is much more readable (so, attach() - and others, detach()).

It was mentioned in Uncle Bob book, useful article here: https://medium.com/@amlcurran/clean-code-the-curse-of-a-boolean-parameter-c237a830b7a3

[glamorous]

I'm fully aware of passing a boolean is not always a good idea. But in this case it's ment to have the "flag" be more dynamic. Like for example based on the execution of closure to determine if it should all be "attached" or "not" without to figure out which one's already exists or not.