Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've created a script to create a module in our MVC framework. The structure of the module is like this (for this example, the module is called test):

test >
    Test.php
    controllers >
        IndexController.php
    models >
        TestModel.php
    open >
       css >
       js >
    views >

Here's what I have for the script. If can have an optional flag of -o to allow the script to remove and override the folder is it already exists.

#!/bin/bash


OVERRIDE=false

while getopts ":o" opt; do
        case $opt in
                o ) OVERRIDE=true
        esac
done


# Check we're in the right directory.

if [ ${PWD##*/} != "modules" ]; then
        echo "Make sure you're in the modules directory"
        exit 1
fi


echo "Enter the module name you want:"

# Get the name, and format it into the formats we need.
read moduleName
lowercaseModuleName=${moduleName,,}
uppercaseFirstModuleName=${lowercaseModuleName^}



# Check if the directory exists already.
if [ -d $lowercaseModuleName ]
    then
        if [ "$OVERRIDE" = true ]; then
                echo "Directory already exists, overridding it because of the -o flag"
                rm -rf $lowercaseModuleName
        else

                echo "Directory '$lowercaseModuleName' already exists, please remove if and try again. To have it overridden please use the -o flag"
                exit 1;
        fi
fi

# Make dir.
echo "Adding module $lowercaseModuleName..."
mkdir $lowercaseModuleName
cd $lowercaseModuleName


# Make root class
echo "Adding Root class..."
cat > $uppercaseFirstModuleName.php << EOF
<?php

class $uppercaseFirstModuleName extends Layer_Shared_Controller {

        public function preDispatch(){

        }
}
EOF




# Make controllers
echo "Adding IndexController..."
mkdir controllers
cd controllers

cat > IndexController.php << EOF
<?php

class IndexController extends $uppercaseFirstModuleName {

        /**
         * Index action.
         */

        public function indexAction(){

        }
}
EOF



# Add model
echo "Adding model..."
cd ../
mkdir models
cd models

cat > $uppercaseFirstModuleName"Model.php" << EOF
<?php

class ${uppercaseFirstModuleName}Model extends Layer_Model {

}
EOF


# Add open folder
echo "Adding open folder..."
cd ../
mkdir open
cd open
mkdir css js
cd ../


# Add view folder
echo "Adding view folder..."
mkdir views
cd ../

echo "Done! Module $uppercaseFirstModuleName created."

Is there anything bad/suboptimal, or that could be done easier?

share|improve this question

2 Answers 2

I would read the module names from command line arguments, so you can create several in one go

./script -o test mock hello_world

After you parse the options, remove them from the positional parameters with

shift $((OPTIND - 1))

Then iterate over the remaining parameters with

for moduleName; do
    # your existing code here.
    # if you still use "cd", make sure you chdir back to the "modules" dir
    #
    # change any "exit 1" to "continue" to stay in the for-loop
done

I would also add a test that the moduleName is valid, something like

if [[ $moduleName == *[^[:alnum:]_]* ]]; then
    echo "invalid module name: contains a non-word character"
fi
share|improve this answer

just a few ideas from the top of my head.

I would define a variable with your start directory at the top of the script, so to that you can always refer to absolute paths in contrast to using cd ../ often. In case you ever change the structure of your script, this will ensure that everything keeps working.

MODULE_DIR=$(readlink -f $lowercaseModuleName)

Now the directory creation can be simplified:

# Add open folder
echo "Adding open folder..."
mkdir -p ${MODULE_DIR}/open/{css,js}/    

# Add views folder
echo "Adding view folder..."
mkdir ${MODULE_DIR}/views

As for the structure, I'd probaly create all the directories in one 'paragraph' and add the templates in a second step, just so that you one place where you can clearly see which directories are being created. But that might just be my personal tast.

In the template creation I'd use absolute paths here as well, using the new MODULE_DIR variable. Keeps you from having to navigate around a lot using cd which might cause confusion later on.

Hope this helps!

share|improve this answer
    
Thanks :), I didn't think about using an absolute path. I'll take a look now at some of the commands I'm not familiar with and update my script with them. –  Tom Hart Nov 13 '14 at 14:43

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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