Welcome to the first entry review for my Advanced ColdFusion contest. Our first entry is from Steve Bryant. I have to say that if every other entry is like this, I will be incredibly happy. You may download it using the Download link below.
As a reminder, I will be picking apart these applications and pointing out things I think are done well, and done badly. This is not meant to attack the author, but merely to give my opinion on how he did things. I am not perfect myself, but one of the points of this exercise is for us to learn from each other. So, enough touchy-feely crap, let's get to it.
Presenting CodeCop

I'm going to begin by commenting on something that isn't ColdFusion related, or even contest related. I currently run a few open source projects, and I think I'm getting the hang of how to do it right. One nit picky thing I saw right out with his zip file is that the install files are intermingled with the rest of his code. I always recommend folks use a "install" subfolder. This makes it easier to find the installation instructions, and makes it easier to remove the install files from a production box. Since install files could potentially be dangerous, it is always a good idea to make it easy to remove them.
My next issue was with the installation instructions. In particular, he mentions that you will be asked to pick a datasource, but he doesn't say why. The application installs itself into a database, but nowhere is that made clear. I was naturally concerned so I first picked some old database. The application did not like that (but handled the error gracefully). I then made a new DSN with an empty database and that made things work nicely. Note to Steve - you simply cannot provide enough information in your instructions. Always provide as much as possible. For example, would any DSN have worked? SQL Server worked, but what about MySQL? As a quick side note - he also provides a "contest_notes.txt" file which talks about his thought process. To be honest, that's pretty cool. None of my applications talk about why I did stuff, the philosophy behind design decisions, etc.
So - after a bit of fuss setting things up, I had the application running. The first thing you are asked to do is to pick a folder of code to scan. You have the option to specify particular extensions or to simply use all extensions. (I assume he meant cfc and cfm and not really .. If he did, he should document that.) You are also prompted to select a rules package. I chose the default. After a few seconds your report shows up.
The report gives you a nice breakdown of all the issues discovered in the code you processed. You can then jump into the file and see the issue, or click on the issue type and get more information about what it means. Reports are automatically stored into the database and you have the option to export to PDF as well. All in all, the UI is very nice and clean. Now let's talk about the engine.

When I created this contest, it was my assumption that folks would extend the rules engine by writing code, much like how I built Canvas. Steve however made the configuration all web based. Rules are added and edited via the interface. Rules have various metadata associated with them so you can assign a severity and other properties. Rules can be either regex or tag based, but here again the lack of documentation makes it a bit difficult to know which to use. For example, for a tag based rule, do I do <cffoo> or just cffoo? Looking at another rule, it seems as if you write it without the brackets. However, my test threw an error when I tried to search for cfquery. You can also write custom code, again via the interface, to do specific checking for the rule. I'd imagine this is a bit hard to test though.
Rules can be placed in packages, which is nice, since you may not like the "Prefer structKeyExists over isDefined" rule. One thing that would have been nice, but potentially complex, is the ability to assign different rule priorities in packages. So for example, I may want to keep the structKeyExists rule, but change the priority in a different package.
Another cool aspect of this application is that it doesn't have to run under the ColdFusion Administrator, and if it doesn't, it actually changes the skin used to display the application.
So, that's it. If you want a sample of the output, check this PDF, it is a report created by running the default rule package on CodeCop itself. All in all I'm very impressed with this product, and by how the extensibility doesn't require you to write code. Again, that isn't what I expected, but I like being surprised. The primary issue he has now is simply documentation. Don't forget that you can download this application below.
Archived Comments
I like the charts, you quickly see what type of errors you have in your report. I should have done things like that in my submission :)
Nice one, slick an clean.
Well I got it installed and running, you mentioned it didn't ask about database type Ray, but when it asked for a DSN there was a drop down right underneath with MS SQL, MySQL, etc.
I like how you can click right into highlited source code to view mistakes, and I really like how when you click on an error, it shows a list of all templates that use that error. I have not made any rules yet, but I really like this so far!
I ran this against one of my projects, and one of the things it noticed right away is my use of the 'evaluate()' function. I know thats generally a no-no, but how else would you get the value for a variably named variable? ie Evaluate("URL.level#Numberformat(i,"0000")#")
Justice,
URL["level#Numberformat(i,'0000')#"]
Justice, I must have missed that. All I remembered was the DSN name. So ignore that comment.
FYI - Steve has a new zip. I have replaced the zip as of RIGHT NOW. If you downloaded, redownload. It fixes the bug I mentioned with a tag rule.
Steve,
Oh duh, because the URL is actually a structure of values, I gotcha.
Thanks man!
Ray,
Thanks for all the feedback! I will try to get my documentation updated soon.
What you see for the DSN choice actually varies based on where the installation is. If CodeCop can find a list of datasources (if it is running on CFMX 6.1 or in the administrator of CFMX 7), then it will return a list of datasources for the supported database types.
If you add a new DataMgr.cfc for a new database type and refresh the code (by setting URL.reinit to 1), then the new database type will be supported (so you could add DataMgr_Ingres.cfc and the whole application would support Ingres).
Nonetheless, I should have documented that and plan to do so.
Justice,
Sorry for the brief answer. Basically all ColdFusion variable scopes are structure and can be accessed with structure notation as given. So, URl.myvar is the same as URL["myvar"].
Wow, Steve, that's a pretty slick implementation you have here. Kudos.
Ray, I think this particular contest idea was really cool - not only as a learning exercise, but also because the resulting applications are really useful.
I know it's bad form to agree with a compliment, but I will. If I had gotten only one entry, I'd be 100% pleased.
As an FYI, next Friday I'll have the second entry.
Is there a compiled list of CF best coding practices somewhere that we could use to make our rules?
Why is StructKeyExists() prefered over isDefined()?
Has anyone got this working with an Access db? I made a new mdb, set up the CF dsn, the tables were set up...but I get an error trying to load the CodeCop homepage (and am so far removed from working with Access that I can't for the live of me remember if the query should work or not)...
Error Executing Database Query.
[Macromedia][SequeLink JDBC Driver][ODBC Socket][Microsoft][ODBC Microsoft Access Driver] Syntax error (missing operator) in query expression 'chkRules.SeverityID = chkSeverities.SeverityID INNER JOIN chkCategories ON chkRules.CategoryID = chkCategories.CategoryID'.
The error occurred in D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Rules.cfc: line 101
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Rules.cfc: line 242
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Transfer.cfc: line 69
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\CodeCop.cfc: line 68
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\Application.cfm: line 73
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Rules.cfc: line 101
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Rules.cfc: line 242
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\Transfer.cfc: line 69
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\sys\CodeCop.cfc: line 68
Called from D:\inetpub\wwwroot\CFIDE\administrator\CodeCop\Application.cfm: line 73
99 : INNER JOIN chkCategories
100 : ON chkRules.CategoryID = chkCategories.CategoryID
101 : WHERE RuleID = <cfqueryparam value="#arguments.RuleID#" cfsqltype="CF_SQL_INTEGER">
102 : </cfquery>
103 :
--------------------------------------------------------------------------------
SQL SELECT RuleID,RuleName,RuleType,TagName,Regex,Description,allExtensions,UUID,CustomCode, chkSeverities.SeverityID,SeverityName,rank, chkCategories.CategoryID,CategoryName, '' AS Packages, '' AS Extensions, '' AS ExtensionNames FROM chkRules INNER JOIN chkSeverities ON chkRules.SeverityID = chkSeverities.SeverityID INNER JOIN chkCategories ON chkRules.CategoryID = chkCategories.CategoryID WHERE RuleID = (param 1)
DATASOURCE CodeCop
VENDORERRORCODE -3100
SQLSTATE 42000
Dan,
Read through:
http://ray.camdenfamily.com...
and
http://corfield.org/blog/in...
for your answer. Surprised me too when Ray enlightened me.
Dan,
Good question about the compiled list of rules. I just made a blog entry to try to compile a list. Head on over there and make your suggestions!
http://steve.coldfusionjour...
Rich,
I know I tested this on Access at some point in development, but I must have made some changes since then. I don't see anything in that SQL that stands out, but maybe I accidentally used a reserved word or something. I will check it out.
Scott,
Thanks, that helps a lot! :-) (It's also disapointing, because I really like the simplicity and readability of isDefined().)
It looks like the issue with Access is that it doesn't support multiple joins in a query without parenthesis to dictate processing order.
Which means that in order to support Access, I have to change all of my pretty inner joins that are cluttered up by nested parenthesis or change to a less clear comma delimited list of tables in the from clause and do the joins in the where clause.
I'm not happy with Access at the moment.
<rant>
RE: The Evaluate() comment in the screenshot.
People always claim Evaluate is slower, and never offer any evidence to back it up.
I've done simple tests myself, comparing thousands of iterations of Evaluate("Form.#Var#") against Form[Var], and there was no notable difference between them.
Obviously Form[Var] is far more elegant, but I get irritated by people making speed claims without checking/offering proof - especially if the advice is being given from an authoritive source, such as a code checking application.
</rant>
Peter, just to be absolutely clear. This contest is NOT about creating a bible of best practices. While that is the _result_, the contest is about the engine itself. I don't want to get into religous wars about best practices. :)
Check this for a view from a core CF engineer:
http://blogs.sanmathi.org/a...
"As has been noted often enough, evaluate() and iif() are not the most performant of functions, since they do need to dynamically compile the expressions that they’re being asked to process."
Fair enough - I'd just been seeing the claim in quite a few places recently and couldn't contain myself any longer. :)
Thanks for that link though, it's a nice explanation, and has a good clear summary on when/not to use them...
> In summary - use evaluate() and iif() in situations
> where the expression to be evaluated remains static over
> time. Using them for dynamic expressions will prove
> inefficient, and may well negate any benefits that
> static expressions enjoy.
This just makes me happy to be a CF programmer. Our crowd is great.
Steve, thank you for sharing.
Ray, a big thank you to you too.
Erm, errors out on me:
cannot convert the value "" to a boolean
The error occurred in D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sys\Reviews.cfc: line 208
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sys\CodeCop.cfc: line 120
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 779
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 756
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 755
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 753
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 326
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 233
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\sebtags\sebForm.cfm: line 233
Called from D:\CFusionMX7\wwwroot\CFIDE\administrator\CodeCop\index.cfm: line 34
206 : <cfloop query="qRules">
207 : <!--- If this is a file that should be checked for this rule --->
208 : <cfif allExtensions OR ListFindNoCase(ExtensionNames,ListLast(ListLast(FileInfo.FullName,"/"),"."))>
209 : <!--- Run check for this rule type --->
210 : <cfset variables.RuleTypes[RuleType].checkRule(ReviewID,RuleID,FileInfo)>
Anyone else had the same problem? Not sure if it's my dodgy cfc's I'm testing or if it's a problem with CodeCop...
Tom,
It looks like a problem with CodeCop. I think I see what caused it.
Would you mind dropping me a line with your information so I can test it in a situation closely matching your?
Either on my web site or send me an email (my first name at my domain name).
http://www.bryantwebconsult...
If anyone else runs into trouble, let me know as well. I am hoping to have the first public beta out soon (feel free to use this copy until then).
Steve,
You may want to ping Steve Nelson (of Fusebox fame). I remember way back in the day, he had a code checking tool as well. Many of us (myself included) sent him ideas for rules at the time. Perhaps he'd be willing to share...
Rob,
That is a good idea. I will do that.
Tom,
I just uploaded a new build to my site that should fix issues with Access. The bad news is that you will have to use a different database or delete the "chk" tables from the one you are using in order for the fixes to be effective (CodeCop will recreate them correctly).
http://www.bryantwebconsult...
I am also hoping to create some real documentation as soon as I get time.
Just as a suggestion, maybe you could make an export/import function on your rule packages, so that people can share good rules they have written? =)
Justice,
Good idea!
Actually, that is there already (even in my original entry).
The packages page has an "Import Package" link that will allow you to import a package from an XML file.
If you edit a package, that page as an "Export Package" link which will export the XML for that package.
This will allow you to share packages and rules with other developers.
I also just put out a new build which fixes the problems with MS Access, improved the installation instructions, and adds CFC Introspection.
http://steve.coldfusionjour...
well, I am going to blame it on the lack of coffee this morning. I am working on a cup right now, and I wont post till its gone =)
Thanks for a very cool product, but I shudder when I run it against code I ran 4 years ago...
Steve,
Great, your update works just fine. A great little application. I just hope it doesn't start handing out fines now. Have enough trouble on the roads.
Tom
I, of course, had to download and try it out. I'm using MySQL - 5.0.17 max. I ran into a serious issue when I clicked on one of the files - got an SQL syntax error message. I think as a general rule, if one is going to support a selected number of databases, these should at least be given an adequate test, and information placed in the documentation as to which database versions have been tested.
Another issue I have with this application is - not enough documentation. What does this do, how can it help me do my job better, convince me why I should use it.
I just uploaded a new build (10) which fixes the MySQL issue. I retested on Access and MS SQL. I haven't been able to test on PostGreSQL though. If anyone runs into trouble on that, I may need access to the database to test in order to fix it.
I know I need documentation for it. I just have to find the time to do that.
Lola, when you got the SQL error, did you make sure the MySQL service was running? The first time I tried it I got an error and was wondering what the hell was going on until I realised the service was started on my server machine. Doh!
Hi Steve,
I just installed the latest version from your site and it's working pretty well. I did notice an error, however, when I click on By Categories, By Files, or By Rules links in the report.
<code>
The column name (FileName_UCase) that you specified already exists in this query.
Column names must be unique.
The error occurred in C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 360
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 352
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 341
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 161
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 161
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\sebtags\sebTable.cfm: line 1
Called from C:\CFusionMX7\wwwroot\tools\CodeCop\review-rules.cfm: line 42
358 : <cfset ArrayAppend(aSortVals,UCase(qTableData[ThisTag.qColumns[i].sortfield][CurrentRow]))>
359 : </cfloop>
360 : <cfset QueryAddColumn(qTableData, "#ThisTag.qColumns[i].sortfield#_UCase", aSortVals)>
361 : </cfif>
362 : </cfloop>
</code>
Rob,
Thanks for the catch.
It looks like the newer version that I had available as an example implementation of DataMgr 2.0 Beta didn't have the error, so I made that the primary download for CodeCop.
As an aside, I haven't forgotten about this project. My spare time has been limited and mostly dedicated to DataMgr 2 lately. Hopefully I will get the install cleaned up and release CodeCop 1.0 in the somewhat near future.
Thanks,
Steve
Access error: Data type mismatch in criteria expression.
Error line: CodeCop\com\sebtools\DataMgr.cfc: line 1522
line 1522: <cfquery name="qQuery" datasource="#variables.datasource#"><cfloop index="i" from="1" to="#ArrayLen(aSQL)#" step="1"><cfif IsSimpleValue(aSQL[i])><cfset temp = aSQL[i]>#Trim(PreserveSingleQuotes(temp))#<cfelseif IsStruct(aSQL[i])><cfset aSQL[i] = queryparam(argumentCollection=aSQL[i])><cfswitch expression="#aSQL[i].cfsqltype#"><cfcase value="CF_SQL_BIT"><cfif aSQL[i].value>1<cfelse>0</cfif></cfcase><cfcase value="CF_SQL_DATE">#CreateODBCDateTime(aSQL[i].value)#</cfcase><cfdefaultcase><cfif ListFindNoCase(variables.dectypes,aSQL[i].cfsqltype)>#Val(aSQL[i].value)#<cfelse><cfqueryparam value="#aSQL[i].value#" cfsqltype="#aSQL[i].cfsqltype#" maxlength="#aSQL[i].maxlength#" scale="#aSQL[i].scale#" null="#aSQL[i].null#" list="#aSQL[i].list#" separator="#aSQL[i].separator#"></cfif></cfdefaultcase></cfswitch></cfif> </cfloop></cfquery>
Thomas,
Ray's year old blog entry probably isn't the best place for support.
The best thing to do is go to the RiaForge page (listed below) and make sure you are running the latest version.
If you still have the bug, you can report it there or go to my site and fill out the contact form and I will be happy to see what I can do to help you out.
http://codecop.riaforge.org/
While installing on CF11, I got following error.
After adding datasourcename and selecting MySQL ...
-------------------------------------------------------------------
The value returned from the StructKeyHasLen function is not of type numeric.
If the component name is specified as a return type, it is possible that either a definition file for the component cannot be found or is not accessible.
The error occurred in D:/sites/test/web/CodeCop/com/sebtools/DataMgr.cfc: line 2022
Called from D:/sites/test/web/CodeCop/sys/CodeCop.cfc: line 33
Called from D:/sites/test/web/CodeCop/Application.cfm: line 69
2020 : }
2021 : //Set alias (if exists)
2022 : if ( StructKeyHasLen(thisField,"alias") ) {
2023 : tmpStruct["alias"] = Trim(thisField["alias"]);
2024 : }
-------------------------------------------------------------------
Any idea?
Change the returntype on the StructKeyHasLen method from "numeric" to "boolean" and refresh the application.
Before ColdFusion 11, any boolean value also counted as a numeric value in a returntype (allowing me to use returntype="numeric" if I expected to return boolean, but wanted to leave my options open). In ColdFusion 11, a boolean does not count as numeric. Which makes sense, but breaks some code.
I'll try to post up an update to DataMgr on this in near future and an update to CodeCop eventually.
I just installed the codecop from this blog and unfortunately i can not work with Coldfusion 10??? I created the datasource in MySQL and i got the message
Could not find the ColdFusion component or interface types.Reviews.
Ensure that the name is correct and that the component or interface exists.
The error occurred in C:/wamp/www/qstools/codecop/sys/Reviews.cfc: line 29
Called from C:/wamp/www/qstools/codecop/sys/CodeCop.cfc: line 132
Called from C:/wamp/www/qstools/codecop/sys/CodeCop.cfc: line 61
Called from C:/wamp/www/qstools/codecop/Application.cfm: line 73
27 : <cfloop query="qRuleTypes">
28 : <cfif name neq "base.cfc">
29 : <cfset variables.RuleTypes[ListFirst(name,".")] = CreateObject("component","types.#ListFirst(name,'.')#").init(variables.Files,variables.Rules,variables.Issues)>
30 : </cfif>
31 : </cfloop>
Any help would convenient to me???
Kind Regards
i followed instructions.txt inside Codecop folder and i have the problem
Element CONFIGSITE is undefined in a CFML structure referenced as part of an expression.
The error occurred in C:/wamp/www/qstools/codecop/Application.cfm: line 45
43 : </cfif>
44 :
45 : <cfset request.Paths = Application["CodeCop"].ConfigSite.getPaths()>
46 :
47 : <cfset SettingsData = Application["CodeCop"].SettingsMgr.getSettings()><!--- Get settings data --
Can you please contact the author of this app? This post is *very* old and the code is probably going to need lots of updating.
You can fill out the form on the RIAForge page or the contact form on my site. I want to get CodeCop updated, but I am pretty behind on client work right now so it will probably be a little while.