Check if relations exists for a parent node before deleting it so that we don't have any orphan child node Here I am trying to delete a parent node but only if it doesn't have any child node. Please review this code.

PostController.php

/**
 * Delete the given post.
 * 
 * @param  int $post_id
 * @return void
 *
 * @throws \App\Exceptions\RelationExistsException
 */
public function deletePost($post_id)
{
    $relations = ['tags', 'comments'];

    $hasRelations = $this-&gt;postRepo-&gt;hasRelations($post_id, $relations);

    if ($hasRelations) {
        throw new RelationExistsException($relations);
    }

    $this-&gt;postRepo-&gt;delete($post_id);
}


RepositoryTrait.php

/**
 * Check if any relation exists.
 *
 * @param  int  $id
 * @param  array  $relations
 * @return bool
 */
public function hasRelations($id, array $relations)
{
    if (count($relations) == 0) {
        throw new \Exception('No relation is provided.');
    }

    $modelName = $this-&gt;model;

    $query = $modelName::where('id', $id);

    $query-&gt;where(function($q) use($relations) {
        foreach ($relations as $relation) {
            $q-&gt;orHas($relation);
        }
    });

    return $query-&gt;exists();
}


RelationExistsException.php

namespace App\Exceptions;

use Exception;

class RelationExistsException extends Exception
{
    /**
    * Create a new exception instance.
    *
    * @param array $relations
    * @return void
    */
    function __construct($relations)
    {
        parent::__construct(sprintf(
            'Cannot delete because there exists relations - %s.', implode(', ', $relations))
        );
    }
}


I have one more issue with the following code:

$modelName = $this-&gt;model; // contains '\App\Models\Post'

$query = $modelName::where('id', $id);


I have to save the $this-&gt;model in $modelName to use it with the where function. Is there a better solution for it. I have tried {}, () but nothing worked.

GitHub Gist for above code
Some thoughts below:

I would encourage you to think about validating parameters more thoroughly on your public methods.  For example, in deletePosts(), you do nothing to validate that you even have an integer value (or whatever) to work with.  You do it in some cases such as type-hinting for array and validating non-empty array on hasRelations(), but it is not consistent.  What if non-array is passed to your exception constructor? What if non-integer (or whatever) is passed to deletePosts()?

You may not think it matters now because you are currently working in this application area and understand where all the calls are made against these methods, but think about in the future when you try to leverage these classes in new ways.  The call patterns may be different.  If, for example, you introduce a buggy use case that passes a non-array to your exception class, you want that class to complain loudly, rather than silently fail so that you can focus your debugging efforts more quickly on the problem code.




$relations = ['tags', 'comments'];



Why is this hard-coded here?  I would think this should, at a minimum, be a property on the class, if not derived from configuration.




$hasRelations = $this-&gt;postRepo-&gt;hasRelations($post_id, $relations);



Consider placing this code in a try-catch block since hasRelations() can throw. I know that since $relations is hard-coded here that we would never expect to get into a state where that exception is thrown, however I think it best practice to always use try-catch block in such a case so that as someone working in this code, you have quick understanding of how methods you are calling might perform.  Who knows, maybe the implementation of RepositoryTrait changes at some point and throws exceptions for other reasons besides the passed relations parameter.




throw new \Exception('No relation is provided.');



Consider throwing InvalidArgumentException if you want to be more specific here.  I actually find it a bit odd that you use custom exception types in the code calling this but not here.  It might call into question how you are using custom exceptions throughout the application.



Your custom exception seems to have a very limited use case. It really only exists to format the message string from a passed array, functionality that might rightfully live in the code where the exception is thrown not within the logic of the exception.

Is this message even meaningful to caller as is?  You can't tell what kind of relationship exists amongst the types provided, nor the specific ID's of the relationships that exist, so there is questionable value in preparing this specific message string vs. just a simple 'Cannot delete because this post has relations' message in the exception. You could easily provide this message in context of where the exception is thrown totally eliminating the need to override the constructor (or maybe even this class altogether).

Is this class going to be used elsewhere in your application?  If not, should it even exist (vs. using other exception types)? If so, are you always going to want to pass this class an array of relation types as parameter to format into message string?

I actually question whether you should even be throwing an exception here at all if a relationship exists.  Since you are specifically building this functionality, my guess is that you are expecting the application to handle deletion requests against posts that have relationships as part of normal operation of the application.  If so, should this really be an exception or just an alternate code path that needs to be followed to handle this condition?  Only if you truly never expect the application to be put into this state would it make sense to have this code throw an exception.



To your question about how to call model dynamically while avoiding having to set $modelName, you could use something like:

call_user_func($this-&gt;model . '::where', 'id', $id);


but I honestly find what you have in place to be easier to read, and I would not be concerned about the cost of having the additional variable in memory (that seems like a micro-optimization type of concern).