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 have the following code, but it somehow looks a bit redundant. As I am new to ColdFusion, I am not sure how to optimize it. Can someone suggest an alternative?

<cfparam name="url.idInfopage" default="0">
<cfif  not isNumeric(url.idInfopage)><cfset url.idInfopage= 0></cfif>

<cfif isDefined("stElement.idInfopage") and isNumeric(stElement.idInfopage)>
    <cfset idInfopage= stElement.idInfopage>
    <cfif  idInfopage is ""><cfset idInfopage= 0></cfif> 
<cfelse>
    <cfset idInfopage= url.idInfopage>
</cfif>
share|improve this question
1  
What version of CF are you running? –  Busches Feb 4 '13 at 16:20
    
I am using MX7 at the moment. –  user160820 Feb 4 '13 at 16:21
add comment

migrated from stackoverflow.com Feb 6 '13 at 19:00

This question came from our site for professional and enthusiast programmers.

1 Answer

up vote 8 down vote accepted

You could re-write it something like this:

<cfif StructKeyExists(stElement,'idInfoPage') AND isNumeric(stElement.idInfopage)>
    <cfset idInfopage = stElement.idInfopage />

<cfelseif StructKeyExists(Url,'idInfoPage') AND isNumeric(Url.idInfopage)>
    <cfset idInfopage = Url.idInfopage />

<cfelse>
    <cfset idInfopage = 0 />

</cfif>

Notes on the changes:


If you have a lot of code like this, it might make sense to create a function, something like:

<cfset idInfopage = getNumberOrZero('idInfopage',stElement,Url) />

Then implemented as:

<cffunction name="getNumberOrZero" returntype="numeric" output=false
    hint="Loops through structs (args 2..n) looking for number; else return 0">
    <cfargument name="Key" type="String" required />
    <cfset var i = 0 />

    <cfloop index="i" from=2 to=#ArrayLen(Arguments)#>
        <cfif StructKeyExists(Arguments[i],Arguments.Key)
            AND isNumeric(Arguments[i][Arguments.Key])
            >
            <cfreturn Arguments[i][Arguments.Key] />
        </cfif>
    </cfloop>

    <cfreturn 0 />
</cffunction>
share|improve this answer
add comment

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.