In today's world, we use frameworks, and separate concerns with MVC or MV*, we strive for Clean Code to make Uncle Bob proud, or Bossman Bob at least. Just because the “Run” function is all you need in a CFC (which extends the BaseCommand) for CommandBox to index the Command, doesn’t mean you have to jam all your code into that one function. Lets look at some simple code cleanup.

On my own blog, we've looked a creating a simple set of commands, which I decided to call KIWI SAYS, which help with creating Host files in Apache, Lucee, and gives you some shortcuts for restarting your engines, but the main command in the set is addWebsite. It is the main controller almost, that does a lot of checking config, and setup, and if everything is good, then we’ll actually do produce the output, and restart apache. It seems like a perfect place to start, because that's a lot of code.

Assuming I had done no cleanup or refactoring, my run function would look something like this.

component extends="commandbox.system.BaseCommand" {
   
    property name="settingsObj";
        
    /**
* @websitePath.hint Path to the Website Directory - .\ for current directory
* @websiteURL.hint The Website URL
* @engine.hint The CFML Engine you want the Website to run on - lucee, railo or cf11
* @engine.options Lucee,Railo,CF11
*/
    function run( required string websitePath, required string websiteURL, required string engine ){
        this.settingsObj = createObject( "component", "set.settings");
       
        ARGUMENTS.websitePath =  fileSystemUtil.resolvePath( ARGUMENTS.websitePath );
        if( !directoryExists( ARGUMENTS.websitePath ) ) {
            return error( "#ARGUMENTS.websitePath#: No such directory - Please enter a valid relative or full path" );
        }
        if ( listFind( "lucee,railo,cf11", ARGUMENTS.engine) == false ){
               error('Please select a valid Engine - Lucee, Railo or CF11’);
        }
        var apacheConf = "";
        apacheConf = apacheConf & '<VirtualHost *:80>' & CR;
        if ( this.settingsObj.get("ApacheAdminEmail") GT ""){
            apacheConf = apacheConf & 'ServerAdmin ' & this.settingsObj.get("ApacheAdminEmail") & CR;
        }
        apacheConf = apacheConf & 'DocumentRoot "#websitePath#"' & CR;
        apacheConf = apacheConf & 'ServerName #websiteURL#' & CR;
        if ( engine == 'CF11'){
            apacheConf = apacheConf & & 'Include "' & this.settingsObj.get("cf11ConnFile") & '"' & CR;
        }
        else {
            apacheConf = apacheConf  & '<Proxy *>' & CR;
            apacheConf = apacheConf & 'Allow from 127.0.0.1' & CR;
            apacheConf = apacheConf & '</Proxy>' & CR;
            apacheConf = apacheConf & 'ProxyPreserveHost On' & CR;
            apacheConf = apacheConf & 'ProxyPassMatch ^/(.+\.cf[cm])(/.*)?$ ajp://localhost:';

            if ( engine == "Railo" ){
                  apacheConf = apacheConf & this.settingsObj.get("railoAJPPort");
            }
            else {
                  apacheConf = apacheConf & this.settingsObj.get("luceeAJPPort");
            }
            apacheConf = apacheConf &  replacenocase( websitePath, this.settingsObj.get("tomcatRootPath"), "" );
            apacheConf = apacheConf & '/$1$2' & CR;
        }
        apacheConf = apacheConf & '</VirtualHost>' & CR;
        print.line().blueLine( apacheConf );
        
        var apacheConfPath = this.settingsObj.get("apacheConfPath");
        print.line( 'apacheConfPath = ' & apacheConfPath );
        if( apacheConfPath == '' ) {
            return error( "#apacheConfPath#: No such directory" );
        }
   
        apacheConfPath = fileSystemUtil.resolvePath( apacheConfPath );
        print.line( 'apacheConfPath = ' & apacheConfPath );
        if( !directoryExists( apacheConfPath ) ) {
            return error( "#apacheConfPath#: No such directory" );
        }
        print.line( 'Writing Conf to File: ' & apacheConfPath & '/' & websiteURL & '.conf' );
        fileWrite( apacheConfPath & '/' & websiteURL & '.conf', apacheConf );
        
         runCommand( "kiwiSays restartApache" );
        
    }
}

 

Can you follow that code?

I wrote it, and to be honest, in this format, its not pretty, and I would not like working on this. Lets make it CLEAN(er) code… and we’ll do that like we would do with any other CFC, refactor a piece out into smaller functions. CommandBox doesn’t limited the number of functions you can have in a CFC, it will only index, and run the “RUN” function when the name of the CFC is typed into the console. Lets break this massive run function into a more management controller.

Note: I could actually add additional CFCs, like Services, and call each one to perform functions, but to make it easier for readers, all the functions I refactor are going into the same main CFC. The only exception, if my settingsObj.cfc which I use to wrap input and output to a json file, to store settings for this suite of tools… so you can set your paths for where CF is installed, where Lucee is installed, what ports etc.

First piece of code

BEFORE:

ARGUMENTS.websitePath =  fileSystemUtil.resolvePath( ARGUMENTS.websitePath );
if( !directoryExists( ARGUMENTS.websitePath ) ) {
        return error( "#ARGUMENTS.websitePath#: No such directory - Please enter a valid relative or full path" );
}

We are validating the WebsitePath, so that sounds like a great function name.

AFTER:

ARGUMENTS.websitePath = validateWebsitePath( ARGUMENTS.websitePath );

NEW FUNCTION:

function validateWebsitePath( websitePath ){
   
    websitePath = fileSystemUtil.resolvePath( ARGUMENTS.websitePath );
    if( !directoryExists( ARGUMENTS.websitePath ) ) { 
        error( "#ARGUMENTS.websitePath#: No such directory - Please enter a valid relative or full path" );
    }
    return websitePath;
   
}

Second piece of code

BEFORE:

if ( listFind( "lucee,railo,cf11", ARGUMENTS.engine) == false ){
        error('Please select a valid Engine - Lucee, Railo or CF11’);
}

We are validating required settings, again a great name, so we’re going to replace this with a new function. The best thing about this, I have a lot more settings I want to validate, so I’m going to add more here, and we do not need to worry about the main function getting more polluted.

AFTER:

validateRequiredSettings( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );

 

NEW FUNCTION:

function validateRequiredSettings( required string websiteURL, required string websitePath, required string engine ){
   
    if ( validateEngine( ARGUMENTS.engine ) != ""){
        return error( validateEngine( ARGUMENTS.engine ) );
   
    }
    return "";
   
}

 

Third piece of code

So far, we only saved ourselves a few lines, the next chunk of code, is where you see the real benefits.

BEFORE:

var apacheConf = "";
   
apacheConf = apacheConf & '<VirtualHost *:80>' & CR;
if ( this.settingsObj.get("ApacheAdminEmail") GT ""){
    apacheConf = apacheConf & 'ServerAdmin ' & this.settingsObj.get("ApacheAdminEmail") & CR;
}
apacheConf = apacheConf & 'DocumentRoot "#websitePath#"' & CR;
apacheConf = apacheConf & 'ServerName #websiteURL#' & CR;
if ( engine == 'CF11'){
    apacheConf = apacheConf & & 'Include "' & this.settingsObj.get("cf11ConnFile") & '"' & CR;
}
else {
    apacheConf = apacheConf  & '<Proxy *>' & CR;
    apacheConf = apacheConf & 'Allow from 127.0.0.1' & CR;
    apacheConf = apacheConf & '</Proxy>' & CR;
    apacheConf = apacheConf & 'ProxyPreserveHost On' & CR;
    apacheConf = apacheConf & 'ProxyPassMatch ^/(.+\.cf[cm])(/.*)?$ ajp://localhost:';
    if ( engine == "Railo" ){
        apacheConf = apacheConf & this.settingsObj.get("railoAJPPort");
    }
    else {
        apacheConf = apacheConf & this.settingsObj.get("luceeAJPPort");
    }
    apacheConf = apacheConf &  replacenocase( websitePath, this.settingsObj.get("tomcatRootPath"), "" ) & '/$1$2' & CR;
}
apacheConf = apacheConf & '</VirtualHost>' & CR;
print.line().blueLine( apacheConf );

 

We are generating Apache conf, essentially, although this one piece of code does a lot of smaller tasks inside of it… so we’re first going to refactor it into 1 function, called generateApacheConf surprisingly enough… then we can refactor those other pieces out again.

AFTER:

var apacheConf = generateApacheConf( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );

Only 1 line, now our Run function is looking much better. Lets look at the generateApacheConf function.

NEW FUNCTION:

function generateApacheConf( required string websiteURL, required string websitePath, required string engine ){
   
    var apacheConf = "";
   
    apacheConf = apacheConf & '<VirtualHost *:80>' & CR;
    apacheConf = apacheConf & getServerAdmin();
    apacheConf = apacheConf & 'DocumentRoot "#websitePath#"' & CR;
    apacheConf = apacheConf & 'ServerName #websiteURL#' & CR;
    apacheConf = apacheConf & getEngineConnection( engine, websitePath );
    apacheConf = apacheConf & '</VirtualHost>' & CR;

    print.line().blueLine( apacheConf );

    return apacheConf;
   
}

Wait, that's not the same code, no, I took the LOGIC out of this function too. Appending to the ApacheConf variable is easy, but if there was a decision to be made, or more complex output, I refactored that out too.

function getServerAdmin(){
    if ( this.settingsObj.get("ApacheAdminEmail") GT ""){
        return 'ServerAdmin ' & this.settingsObj.get("ApacheAdminEmail") & CR;
    }
    else {
        return '';
    }
}

This getServerAdmin function hides the task of determining if we need a ServerAdmin line in our config, and how to output it from our settings.

 

function getEngineConnection( engine, websitePath ){
    var returnConf = '';
   
    if ( engine == 'CF11'){
        returnConf = returnConf & 'Include "' & this.settingsObj.get("cf11ConnFile") & '"' & CR;
    }
    else {
        returnConf = returnConf & '<Proxy *>' & CR;
        returnConf = returnConf & 'Allow from 127.0.0.1' & CR;
        returnConf = returnConf & '</Proxy>' & CR;
        returnConf = returnConf & 'ProxyPreserveHost On' & CR;
        returnConf = returnConf & 'ProxyPassMatch ^/(.+\.cf[cm])(/.*)?$ ajp://localhost:';
        returnConf = returnConf & getEnginePort( engine );
        returnConf = returnConf & getProxyPath( websitePath ) & '/$1$2' & CR;

    }
    return returnConf;
}

getEngineConnection has a lot of complexity, and that too actually has some refactored functions too. The sub functions hide the complexity from the parent functions… so you can quickly and easily determine what the code is doing. The function names are self commenting, and if there is anything that doesn’t make sense, or is doing too much, I pull it out into its own function. getEnginePort and getProxyPath are both helper functions, but they make the getEngineConnection function much easier to read.

function getEnginePort( engine ){
    if ( engine == "Railo" ){
        return this.settingsObj.get("railoAJPPort");
    }
    else {
        return this.settingsObj.get("luceeAJPPort");
    }
}

function getProxyPath( required string websitePath ) {
    return replacenocase( websitePath, this.settingsObj.get("tomcatRootPath"), "" );
}

Last piece of Code

BEFORE:

var apacheConfPath = this.settingsObj.get("apacheConfPath");
print.line( 'apacheConfPath = ' & apacheConfPath );
if( apacheConfPath == '' ) {
    return error( "#apacheConfPath#: No such directory" );
}
   
apacheConfPath = fileSystemUtil.resolvePath( apacheConfPath );
print.line( 'apacheConfPath = ' & apacheConfPath );
if( !directoryExists( apacheConfPath ) ) {
    return error( "#apacheConfPath#: No such directory" );
}
print.line( 'Writing Conf to File: ' & apacheConfPath & '/' & websiteURL & '.conf' );
fileWrite( apacheConfPath & '/' & websiteURL & '.conf', apacheConf );

Which outputs the Conf to a File so we’ll make a function called outputConfToFile. 

NEW FUNCTION:

function outputConfToFile( required string apacheConf, required string websiteURL ){
    var apacheConfPath = this.settingsObj.get("apacheConfPath");
    print.line( 'apacheConfPath = ' & apacheConfPath );
    if( apacheConfPath == '' ) {
        return error( "#apacheConfPath#: No such directory" );
    }
   
    apacheConfPath = fileSystemUtil.resolvePath( apacheConfPath );
    print.line( 'apacheConfPath = ' & apacheConfPath );
    if( !directoryExists( apacheConfPath ) ) {
        return error( "#apacheConfPath#: No such directory" );
    }
    print.line( 'Writing Conf to File: ' & apacheConfPath & '/' & websiteURL & '.conf' );
    fileWrite( apacheConfPath & '/' & websiteURL & '.conf', apacheConf );
}

Looking at this function, I already want to break it up into multiple functions, but for now I will leave it as one function… as we have done a lot of work for one post.
Lets look at our main run function now.

function run( required string websitePath, required string websiteURL, required string engine ){
        this.settingsObj = createObject( "component", "set.settings");
       
        ARGUMENTS.websitePath = validateWebsitePath( ARGUMENTS.websitePath );
        validateRequiredSettings( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );
        var apacheConf = generateApacheConf( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );
       outputConfToFile( apacheConf, websiteURL );
       runCommand( "kiwiSays restartApache" );
}

Sharp, clean, simple, and to the point.
If you need to know what it does, you can see that from the code, if you want to learn more, you can peel back a layer.
Writing this, I can already see what could or should be broken down more… which is a good sign I’m on the right path.
Just because you are writing a command, doesn’t mean it cannot be clean code.

Full File is here for reference:

component extends="commandbox.system.BaseCommand" {
   
    property name="settingsObj";
        
    /**
    * @websitePath.hint Path to the Website Directory - .\ for current directory  
    * @websiteURL.hint The Website URL
    * @engine.hint The CFML Engine you want the Website to run on - lucee, railo or cf11
    * @engine.options Lucee,Railo,CF11
    */
    function run( required string websitePath, required string websiteURL, required string engine ){
        this.settingsObj = createObject( "component", "set.settings");
       
        ARGUMENTS.websitePath = validateWebsitePath( ARGUMENTS.websitePath );
        validateRequiredSettings( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );
        var apacheConf = generateApacheConf( ARGUMENTS.websiteURL, ARGUMENTS.websitePath, ARGUMENTS.engine );
        outputConfToFile( apacheConf, websiteURL );
        runCommand( "kiwiSays restartApache" );
    }
   
    function validateWebsitePath( websitePath ){
        websitePath = fileSystemUtil.resolvePath( ARGUMENTS.websitePath );
        if( !directoryExists( ARGUMENTS.websitePath ) ) {
             error( "#ARGUMENTS.websitePath#: No such directory - Please enter a valid relative or full path" );
        }
        return websitePath;  
    }
   
    function validateRequiredSettings( required string websiteURL, required string websitePath, required string engine ){
        if ( validateEngine( ARGUMENTS.engine ) != ""){
            return error( validateEngine( ARGUMENTS.engine ) );
        }
        return "";
    }
   
    function validateEngine( engine ){
        if ( listFind( "lucee,railo,cf11", ARGUMENTS.engine) == false ){
            return 'Please select a valid Engine - Lucee, Railo or CF11';
        }
    }
   
    function generateApacheConf( required string websiteURL, required string websitePath, required string engine ){
        var apacheConf = "";
   
        apacheConf = apacheConf & '<VirtualHost *:80>' & CR;
        apacheConf = apacheConf & getServerAdmin();
        apacheConf = apacheConf & 'DocumentRoot "#websitePath#"' & CR;
        apacheConf = apacheConf & 'ServerName #websiteURL#' & CR;
        apacheConf = apacheConf & getEngineConnection( engine, websitePath );
        apacheConf = apacheConf & '</VirtualHost>' & CR;

        print.line().blueLine( apacheConf );
        return apacheConf;
    }
   
    function outputConfToFile( required string apacheConf, required string websiteURL ){
       var apacheConfPath = this.settingsObj.get("apacheConfPath");
       print.line( 'apacheConfPath = ' & apacheConfPath );
       if( apacheConfPath == '' ) {
           return error( "#apacheConfPath#: No such directory" );
       }
       apacheConfPath = fileSystemUtil.resolvePath( apacheConfPath );
        print.line( 'apacheConfPath = ' & apacheConfPath );
        if( !directoryExists( apacheConfPath ) ) {
             return error( "#apacheConfPath#: No such directory" );
        }
        print.line( 'Writing Conf to File: ' & apacheConfPath & '/' & websiteURL & '.conf' );
        fileWrite( apacheConfPath & '/' & websiteURL & '.conf', apacheConf );
    }
   
    function getEngineConnection( engine, websitePath ){
       var returnConf = '';
        if ( engine == 'CF11'){
            returnConf = returnConf & 'Include "' & this.settingsObj.get("cf11ConnFile") & '"' & CR;
        }
        else {
            returnConf = returnConf & '<Proxy *>' & CR;
            returnConf = returnConf & 'Allow from 127.0.0.1' & CR;
            returnConf = returnConf & '</Proxy>' & CR;
            returnConf = returnConf & 'ProxyPreserveHost On' & CR;
            returnConf = returnConf & 'ProxyPassMatch ^/(.+\.cf[cm])(/.*)?$ ajp://localhost:';
            returnConf = returnConf & getEnginePort( engine );
            returnConf = returnConf & getProxyPath( websitePath ) & '/$1$2' & CR;
        }
        return returnConf;
    }
   
    function getEnginePort( engine ){
        if ( engine == "Railo" ){
            return this.settingsObj.get("railoAJPPort");
        }
        else {
            return this.settingsObj.get("luceeAJPPort");
        }
    }
   
    function getServerAdmin(){
        if ( this.settingsObj.get("ApacheAdminEmail") GT ""){
            return 'ServerAdmin ' & this.settingsObj.get("ApacheAdminEmail") & CR;
        }
        else {
            return '';
        }
    }
   
    function getProxyPath( required string websitePath ) {
        return replacenocase( websitePath, this.settingsObj.get("tomcatRootPath"), "" );
    }
}