WORST PRACTICES

 

Classic CF Coding Mistakes

OR…

HOW TO TELL IF YOU SUCK

 

OR…

DO AS I SAY, NOT AS I DO

 

By Tom Lommel

Twin Cities CFUG

March 3, 2004

Example #1

 

<cfset iThisPageSecLevel = 10>

<cfinclude template="incCheckSecLevel.cfm">

<cfparam name="form.txtOldPassword" default="">

<cfparam name="vcOldPassword" default="#form.txtOldPassword#"> 

<cfif isdefined("form.hidFormSubmitted")>

      <cfif len(vcOldPassword) eq 0>

        <cfset vcMessage = vcMessage & "You must enter your current password.<br>">

      </cfif>

      <CFQUERY datasource="#dsn#" name="qryCheckPassword">

          SELECT tblUsers.iUserID

          FROM tblUsers

          WHERE ((tblUsers.bActive)<>0)

            AND tblUsers.iUserID = #request.iMyUserID#

            AND ((tblUsers.vcPassword)=<cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#vcOldPassword#" maxlength="25">)

      </CFQUERY>

      <cfif qryCheckPassword.recordcount eq 0>

        <cfset vcMessage = vcMessage & "The correct current password was not supplied.<br>">

      </cfif>

      <cfif len(vcMessage) eq 0>

        <CFQUERY datasource="#dsn#" name="qryCheckEmail">

          Update tblUsers

          SET vcPassword = <cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#vcNewPassword#" maxlength="25">

          WHERE ((tblUsers.bActive)<>0)

            AND tblUsers.iUserID = #request.iMyUserID#

        </CFQUERY>

        <cfset vcMessage = vcMessage & "Your password has been changed.">

      </cfif>

    </cfif>

Example #1: Failure to comment code

 

<!--- Check security level --->

<cfset iThisPageSecLevel = 10>

<cfinclude template="incCheckSecLevel.cfm">

<!---Default form parameters --->

<cfparam name="form.txtOldPassword" default="">

<cfparam name="vcOldPassword" default="#form.txtOldPassword#"> 

<!--- Are we receiving a form post? --->

<cfif isdefined("form.hidFormSubmitted")>

      <!--- Make sure they passed in their old password --->

      <cfif len(vcOldPassword) eq 0>

        <cfset vcMessage = vcMessage & "You must enter your current password.<br>">

      </cfif>

      <!--- See if their old password matches what’s in the database --->

      <CFQUERY datasource="#dsn#" name="qryCheckPassword">

          SELECT tblUsers.iUserID

          FROM tblUsers

          WHERE ((tblUsers.bActive)<>0)

            AND tblUsers.iUserID = #request.iMyUserID#

            AND ((tblUsers.vcPassword)=<cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#vcOldPassword#" maxlength="25">)

      </CFQUERY>

      <!--- Throw an error message if it does not match--->

      <cfif qryCheckPassword.recordcount eq 0>

        <cfset vcMessage = vcMessage & "The correct current password was not supplied.<br>">

      </cfif>

      <!--- No error message? --->

      <cfif len(vcMessage) eq 0>

        <!--- Update the database --->

        <CFQUERY datasource="#dsn#" name="qryCheckEmail">

          Update tblUsers

          SET vcPassword = <cfqueryparam cfsqltype="CF_SQL_VARCHAR" value="#vcNewPassword#" maxlength="25">

          WHERE ((tblUsers.bActive)<>0)

            AND tblUsers.iUserID = #request.iMyUserID#

        </CFQUERY>

        <!--- Tell the user the change was successful--->

        <cfset vcMessage = vcMessage & "Your password has been changed.">

      </cfif>

    </cfif>

Example #2

 

<cfif isdefined("form.vcfn")>

    <cfset vcfn = form.vcfn >

</cfif>

<cfif isdefined("form.vcln")>

    <cfset vcln = form.vcln >

</cfif>

<cfif isdefined("form.vcad1")>

    <cfset vcad1 = form.vcad1 >

</cfif>

<cfif isdefined("form.vcad2")>

    <cfset vcad2 = form.vcad2 >

</cfif>

<cfif isdefined("form.vccty")>

    <cfset vccty = form.vccty >

</cfif>

<cfif isdefined("form.vcst")>

    <cfset vcst = form.vcst >

</cfif>

<cfif isdefined("form.vczip")>

    <cfset vczip = form.vczip >

</cfif>

<cfif isdefined("form.vcctr")>

    <cfset vcctr = form.vcctr >

</cfif>

Example #2: Vague, Typeless Variable Names

 

<cfif isdefined("form.vcFirstName")>

    <cfset vcFirstName = form.vcFirstName >

</cfif>

<cfif isdefined("form.vcMiddleInit")>

    <cfset vcMiddleInit = form.vcMiddleInit >

</cfif>

<cfif isdefined("form.vcLastName")>

    <cfset vcLastName = form.vcLastName >

</cfif>

<cfif isdefined("form.vcDegree")>

    <cfset vcDegree = form.vcDegree >

</cfif>

<cfif isdefined("form.vcAddress1")>

    <cfset vcAddress1 = form.vcAddress1 >

</cfif>

<cfif isdefined("form.vcAddress2")>

    <cfset vcAddress2 = form.vcAddress2 >

</cfif>

<cfif isdefined("form.vcCity")>

    <cfset vcCity = form.vcCity >

</cfif>

<cfif isdefined("form.vcState")>

    <cfset vcState = form.vcState >

</cfif>

<cfif isdefined("form.vcZip")>

    <cfset vcZip = form.vcZip >

</cfif>

<cfif isdefined("form.vcCountry")>

    <cfset vcCountry = form.vcCountry >

</cfif>

Example #3: Unnecessary Use of # Signs

 

<cfif isdefined("qry_checkDiscount") and #qry_checkDiscount.recordcount# eq 1 and #qry_checkDiscount.dclPercentage# lte 1>

          <cfset #fltPaymentAmount# = #fltPaymentAmount# * (1 - #qry_checkDiscount.dclPercentage#)>

</cfif>

<cfset #fltPaymentAmount# = #NumberFormat(#fltPaymentAmount#, "_.00")#>

 

versus

 

<cfif isdefined("qry_checkDiscount") and qry_checkDiscount.recordcount eq 1 and qry_checkDiscount.dclPercentage lte 1>

          <cfset fltPaymentAmount = fltPaymentAmount * (1 - qry_checkDiscount.dclPercentage)>

</cfif>

<cfset fltPaymentAmount = NumberFormat(fltPaymentAmount, "_.00")>

 

Example #4: Not Locking the Shared Scopes

 

<cfset session.iMyUserID = query.iUserID>

<cfset session.iMySecLevel = 10>

 

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblContent.dtLastUpdated, tblContent.vcShortDesc, tblContent.vcTitle, tblContent.iContentID, tblContent.vcContent, tblContent.iCategoryID, tblCategories.vcCategoryName, tblContent.bActive

FROM         tblContent

WHERE    tblContent.iAuthorID = #session.iMyUserID#

ORDER BY tblCategories.vcCategoryName, tblContent.iContentID DESC

</cfquery>

 

versus

 

<cflock scope="SESSION" type="EXCLUSIVE" timeout="30">

<cfset session.iMyUserID = query.iUserID>

<cfset session.iMySecLevel = 10>

</cflock>

 

<cflock scope="SESSION" type="READONLY timeout="30">

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblContent.vcTitle, tblContent.iContentID

FROM         tblContent

WHERE    tblContent.iAuthorID = #session.iMyUserID#

</cfquery>

</cflock>
Example #5: CFCOOKIE/CFLOCATION combination

 

<CFCOOKIE name="bLoggedIn" value="true">

<CFLOCATION url="welcome.cfm">

 

Example #6: CFELSEIF vs. CFSWITCH

 

<cfif vcState eq "MN">

   <cfset fltTax = .065>

<cfelseif vcState eq "WN">

   <cfset fltTax = .055>

<cfelseif vcState eq "IA">

   <cfset fltTax = .035>

<cfelseif vcState eq "CA">

   <cfset fltTax = .08>

<cfelseif vcState eq "ND">

   <cfset fltTax = .045>

<cfelseif vcState eq "SD">

    <cfset fltTax = .05>

</cfif>

 

versus

 

<cfswitch expression="#vcState#">

          <cfcase value="MN">

                    <cfset fltTax = .065>

          </cfcase>

          <cfcase value="WN">

                    <cfset fltTax = .055>

          </cfcase>

          <cfcase value="IA">

                    <cfset fltTax = .035>

          </cfcase>

          <cfcase value="CA">

                    <cfset fltTax = .08>

          </cfcase>

          <cfcase value="ND">

                    <cfset fltTax = .045>

          </cfcase>

          <cfcase value="SD">

                    <cfset fltTax = .05>

          </cfcase>

</cfswitch>

Example #7: Accepting Blind Data

 

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblContent.vcTitle, tblContent.iContentID

FROM         tblContent

WHERE    tblContent.iAuthorID = #url.iAuthorID#

</cfquery>

 

versus

 

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblContent.vcTitle, tblContent.iContentID

FROM         tblContent

WHERE    tblContent.iAuthorID = <cfqueryparam cfsqltype="CF_SQL_INTEGER" value="#url.iAuthorID#">

</cfquery>

 

Example #8

 

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblCategories.iCategoryID, tblCategories.vcCategoryName

FROM         tblCategories

WHERE     (tblCategories.bActive <> 0)

ORDER BY tblCategories.vcCategoryName

</cfquery>

<table>

<cfloop query="qryGetCategories">

          <cfoutput><tr><th colspan="2">#vcCategoryName#</th></tr></cfoutput>

          <cfquery datasource="#dsn#" name="qryGetContent">

          SELECT          tblContent.dtLastUpdated, tblContent.vcTitle

          FROM          tblContent

          WHERE          tblContent.iCategoryID = #qryGetCategories.iCategoryID#

          </cfquery>

          <cfloop query="qryGetContent">

                    <cfoutput><tr><td>#qryGetContent.vcTitle#</td><td>#qryGetContent.dtLastUpdated#</td></tr></cfoutput>

          </cfloop>

</cfloop>

</table>

Example #8: Making CF Work Hard When the Database Does It Better

 

<cfquery datasource="#dsn#" name="qryGetContentList">

SELECT     tblContent.vcTitle, tblContent.dtLastUpdated, tblCategories.vcCategoryName

FROM         tblContent INNER JOIN tblCategories ON tblContent.iCategoryID = tblCategories.iCategoryID

WHERE     (tblCategories.bActive <> 0)

ORDER BY tblCategories.vcCategoryName

</cfquery>

<table>

<cfoutput query="qryGetContentList" group="vcCategoryName">

   <tr><th colspan="2">#qryGetContentList.vcCategoryName#</th></tr>

   <cfoutput>

      <tr>

        <td>#qryGetContentList.vcTitle#</td><td>#qryGetContentList.dtLastUpdated#</td>

      </tr>

   </cfoutput>

</cfoutput>

</table>

 

Example #9: Making the Database Work Hard When It Doesn’t Have To.

 

<cfquery datasource="#QESdsn#" name="qry_GetStates">

Select vcStateName, vcAbbreviation from tblStates

order by vcAbbreviation

</cfquery>

 

versus

 

<cfquery datasource="#QESdsn#" name="qry_GetStates" cachedwithin="#createtimespan(0,2,0,0)#">

Select vcStateName, vcAbbreviation from tblStates

order by vcAbbreviation

</cfquery>

Example #10: Not Scoping Your Variables

 

What’s the output on the page?

 

<cfif isdefined(“form.vcState”)>

          <cfset vcState = form.vcState>

</cfif>

 

<cfif isdefined(“url.vcState”)>

          <cfset vcState = url.vcState>

</cfif>

 

<cfquery datasource="#QESdsn#" name="qryGetState">

Select vcStateName as vcState, vcAbbreviation from tblStates

Where vcAbbreviation = #session.vcState#

</cfquery>

 

<cfoutput query=”qryGetState”>

#vcState#

</cfoutput>

 

<cfoutput>

#vcState#

<cfoutput>

 

Example #10: Failing to Reuse Code

 

<cfset vcTitle = replace(vcTitle, " ", "&nbsp;", "ALL")>

<cfset vcTitle = replace(vcTitle, "<", "&lt;", "ALL")>

<cfset vcTitle = replace(vcTitle, ">", "&gt;", "ALL")>

<cfset vcNotes = replace(vcNotes, " ", "&nbsp;", "ALL")>

<cfset vcNotes = replace(vcNotes, "<", "&lt;", "ALL")>

<cfset vcNotes = replace(vcNotes, ">", "&gt;", "ALL")>

<cfset vcAddress1 = replace(vcAddress1, " ", "&nbsp;", "ALL")>

<cfset vcAddress1 = replace(vcAddress1, "<", "&lt;", "ALL")>

<cfset vcAddress1 = replace(vcAddress1, ">", "&gt;", "ALL")>

 

versus

 

<cfset vcTitle = fxSafeFormat(vcTitle)>

<cfset vcNotes = fxSafeFormat(vcNotes)>

<cfset vcAddress1 = fxSafeFormat(vcAddress1)>

Example #11: Overuse of Evaluate

 

<cfloop list="#vcFieldList#" index="iThisItem">

          <cfif isdefined("form.#iThisItem#")>

                    <cfset “session.#iThisItem#” = Evaluate(“form.#iThisItem#”)>

          </cfif>

</cfloop>

 

Other Mistakes To Avoid

 

·        ‘Spreadsheet’ Database Design

 

·        Overuse of <CFOUTPUT>

 

·        Not knowing your target platform

 

·        Not TESTING

 

·        Putting too much faith or too big of a burden on the URL

 

·        Not ERROR HANDLING

 

·        Using meaningful data as the primary key in a database