How NOT to Code - Example 2

Jan 09, 2010

Second challenge is below..


<select name="cexpyear">
<option value="00" >cfif cexpyear eq "00"<selected>/cfif<>00
<option value="01" >cfif cexpyear eq "01"<selected>/cfif<>01
<option value="02" >cfif cexpyear eq "02"<selected>/cfif<>02
<option value="03" >cfif cexpyear eq "03"<selected>/cfif<>03
<option value="04" >cfif cexpyear eq "04"<selected>/cfif<>04
<option value="05" >cfif cexpyear eq "05"<selected>/cfif<>05
<option value="06" >cfif cexpyear eq "06"<selected>/cfif<>06
<option value="07" >cfif cexpyear eq "07"<selected>/cfif<>07
<option value="08" >cfif cexpyear eq "08"<selected>/cfif<>08
<option value="01" >cfif cexpyear eq "09"<selected>/cfif<>09
<option value="10" >cfif cexpyear eq "10"<selected>/cfif<>10
<option value="11" >cfif cexpyear eq "11"<selected>/cfif<>11
<option value="12" >cfif cexpyear eq "12"<selected>/cfif<>12
</select>

Comments

Dave Ferguson

Dave Ferguson wrote on 01/11/1011:07 AM

I would just simplify the code and put the options into a loop. I would post the code to do it but I am not sure it would make it through. Also, the var cexpyear is not scoped.



--Dave
John Sieber

John Sieber wrote on 01/13/101:06 AM

Not only could you loop through the option values but if the cexpyear varialbe is being returned from a query object you could select that item by using the selected attribute of the cfselect tag. I would not use the conditional statements inside of the select but if you were going to use them the quotes are not needed around the values as they are numeric.

PS. The captcha text is terribly difficult to read. I do love the concept of these posts though!
John Mason

John Mason wrote on 01/13/101:22 PM

Sorry for the captcha. It really reduces the spam so hopefully it's not too big of a problem.

I usually refer to this type of code problem as a 'time bomb'. Because this is code from 10 years ago. It was written to work up to a certain year and then at some point it won't work correctly for the end users and force a coder to go back and fix the issue. The key here is to code the year loop to work indefinitely with the use of the now() function. So any loop that goes from this year to say 5 years in the future should do the trick.
John Sieber

John Sieber wrote on 01/13/102:33 PM

Good point on the fact that the code will need to be maintained in the future. using the now() function plus a base amount of years makes total sense. No worries on the captcha either. I'm wondering how well cfformprotect works against spam but my blog is way too new to find out as it has little to no traffic.

Write your comment



(it will not be displayed)