How NOT to Code - Example 6

Jan 20, 2010

Can you find what's wrong with this code?


<cfif form.time eq "s1"><cfset form.time = "s7"></cfif>
<cfif form.time eq "s2"><cfset form.time = "s8"></cfif>
<cfif form.time eq "s3"><cfset form.time = "s9"></cfif>
<cfif form.time eq "s4"><cfset form.time = "s10"></cfif>
<cfif form.time eq "s5"><cfset form.time = "s11"></cfif>
<cfif form.time eq "s6"><cfset form.time = "s12"></cfif>
<cfif form.time eq "s7"><cfset form.time = "s13"></cfif>
<cfif form.time eq "s8"><cfset form.time = "s14"></cfif>
<cfif form.time eq "s9"><cfset form.time = "s15"></cfif>
<cfif form.time eq "s10"><cfset form.time = "s16"></cfif>
<cfif form.time eq "s11"><cfset form.time = "s17"></cfif>
<cfif form.time eq "s12"><cfset form.time = "s18"></cfif>
<cfif form.time eq "s13"><cfset form.time = "s19"></cfif>
<cfif form.time eq "s14"><cfset form.time = "s20"></cfif>
<cfif form.time eq "s15"><cfset form.time = "s21"></cfif>
<cfif form.time eq "s16"><cfset form.time = "s22"></cfif>
<cfif form.time eq "s17"><cfset form.time = "s23"></cfif>
<cfif form.time eq "s18"><cfset form.time = "s24"></cfif>
<cfif form.time eq "s19"><cfset form.time = "s25"></cfif>
<cfif form.time eq "s20"><cfset form.time = "s26"></cfif>
<cfif form.time eq "s21"><cfset form.time = "s27"></cfif>
<cfif form.time eq "s22"><cfset form.time = "s28"></cfif>
<cfif form.time eq "s23"><cfset form.time = "s29"></cfif>
<cfif form.time eq "s24"><cfset form.time = "s30"></cfif>
<cfif form.time eq "s25"><cfset form.time = "s31"></cfif>
<cfif form.time eq "s26"><cfset form.time = "s32"></cfif>
<cfif form.time eq "s27"><cfset form.time = "s33"></cfif>
<cfif form.time eq "s28"><cfset form.time = "s34"></cfif>
<cfif form.time eq "s29"><cfset form.time = "s35"></cfif>
<cfif form.time eq "s30"><cfset form.time = "s36"></cfif>
<cfif form.time eq "s31"><cfset form.time = "s37"></cfif>
<cfif form.time eq "s32"><cfset form.time = "s38"></cfif>
<cfif form.time eq "s33"><cfset form.time = "s39"></cfif>
<cfif form.time eq "s34"><cfset form.time = "s40"></cfif>
<cfif form.time eq "s35"><cfset form.time = "s41"></cfif>
<cfif form.time eq "s36"><cfset form.time = "s42"></cfif>

Comments

Steve

Steve wrote on 01/20/103:41 PM

Just by the simple order of things, this makes no sense. By following the above sequence, an "s1" would be an "s37" when done with this sequence.
John Mason

John Mason wrote on 01/20/104:34 PM

Bingo, that's the core problem. Any recommended solutions?
David

David wrote on 01/20/106:13 PM

There are lots of "solutions" :-)

Based on the assumption that it's not *meant* to update the value more than one time 3 that spring to mind are:
- Manipulate the string EG: get the number from the string, add six to it, prefix the result with "s"
- use "elseif"s
- use a "switch/case" statement

Is this code you have actually seen/written?
It's pretty horrible to look at isn't it?!
Jeffry Houser

Jeffry Houser wrote on 01/20/106:43 PM

A case statement w/ breaks would prevent that logic issue that Steve mentioned.

We could also write an algorithm to use some string parsing to pull out the number, add 6 to it and put it back in. But, we don't have enough context of the app from the code segment to tell whether this would work or no.
Steve

Steve wrote on 01/20/109:43 PM

My proposed three line solution:

<cfset timeReplace = ReplaceNoCase(form.time, "s", "", "ALL">
<cfset timeReplace = timeReplace + 6>
<cfset timeReplace = "s" & timeReplace>

Wouldn't say that a loop is necessary, since CF is a typeless language. Moving forward, just use your newly created timeReplace in place of Form.time.

Write your comment



(it will not be displayed)