Search code examples
phpsymfonysymfony-console

is it bad practice to kill a script inside of a function?


I'm referring to using the die() function for something else than debugging. This is a "well it works" situation, but is it bad practice?

use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Filesystem\Exception\IOExceptionInterface;

/**
 * This Command will help Cjw create a new demo site to start off
 * with the Multisite bundle.
 *
 * Class CreateSiteCommand
 * @package Cjw\GeneratorBundle\Command
 */
class RemoveSiteCommand extends ContainerAwareCommand
{
    private $vendor; //eg "Cjw"
    private $fullBundleName; //eg "SiteCjwNetworkBundle"
    private $fullBundleNameWithVendor; //eg "CjwSiteCjwNetworkBundle"
    (more vars)

    /**
     * this function is used to configure the Symfony2 console commands
     */
    protected function configure()
    {
        $this
            ->setName('cjw:delete-site')
            ->setDescription('Delete Cjw Site')
            ->addOption('db_user', null, InputOption::VALUE_REQUIRED, 'If set, the database user will be shown on the instructions');
    }

    /**
     * This function executes the code after the command input has been validated.
     *
     * @param InputInterface $input gets the user input
     * @param OutputInterface $output shows a message on the command line
     * @return int|null|void
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        // set optional db_username

        $dialog = $this->getHelper('dialog');
        $reusable = new \Reusable();
        $fs = new Filesystem();
        $cmd = new \ColorOutput();

        //push only vendors into the vendors array
        $vendors = $reusable->getFileList($reusable->getMainDirectory('src'),true);


        //select a vendor from the array
        $vendor = $dialog->select(
            $output,
            'Select your vendor [1]: ',
            $vendors,
            1
        );

        $bundles_in_vendor = $reusable->getFileList($reusable->getMainDirectory('src/'.$vendors[$vendor]),true);

        //push bundles that start with 'Site' into array
        $sites = array();

        foreach($bundles_in_vendor as $bundle)
        {
            if($reusable->startsWith($bundle,'Site'))
            {
                array_push($sites,$bundle);
            }
        }

        $site_to_delete = $dialog->select(
            $output,
            'Select site to remove: ',
            $sites,
            1
        );

        $bundle_deletion_path = $reusable->getMainDirectory('src/'.$vendors[$vendor]).'/'.$sites[$site_to_delete];

        $are_you_sure = array('yes','no');
        $confirmation = $dialog->select(
            $output,
            'Are you sure you want to delete: '.$sites[$site_to_delete],
            $are_you_sure,
            1
        );

        if($are_you_sure[$confirmation] == 'yes')
        {
            echo $cmd->color('yellow','attempting to remove bundle in: '.$bundle_deletion_path);
            $fs->remove($bundle_deletion_path);

            //returns demo
            $sitename = strtolower($sites[$site_to_delete]);
            $sitename = substr($sitename,0,-6);
            $sitename = substr($sitename,4);
            $this->setRawSiteNameInput($sitename);

            // returns acmedemo
            $symlinkname =  strtolower($vendors[$vendor].substr($sites[$site_to_delete],0,-6));
            $this->removeSymlinks($symlinkname,$this->getRawSiteNameInput());

            $this->createSetters($vendor,substr($sites[$site_to_delete],0,-6));

            $this->deleteEzPublishExtension();
            echo $this->getFullLegacyPath();

            echo $cmd->color('green','deletion process completed.');
        }
        else
        {
            echo "deletion canceled";
            die();
        }

        function_that_further_deletion_process();
    }

This is a symfony2 console script that removes a site from a certain structure


Solution

  • That is perfectly safe, if that is your question, since php as a quasi interpreted language does not leave any traces or artifacts when terminating the execution.

    If it is a good practice is another thing. I'd say it is fine for testing purposes, but you should avoid it in final code. Reason is that it makes code hard to maintain. Consider someone else diving into your code and trying to understand the logic. One would actually have to go through all the code to stumble over this detail. Chances is one does not, so the behavior of your code appears broken.

    Instead try to do one of these:

    • throw an exception to leave the current scope. Such an exception might well be caught and swallowed by the calling scope, but it is a clear and predictable way of returning. Obviously you should document such behavior.

    • return a value clearly out of scope to what would "normally" be returned. So for example null or false instead of a typical value. This forces the calling scope to check the return value, but that is good practice anyway.

    • restructure your code such that there is no reason to suddenly terminate the execution.