How NOT to Code - Example 1
I recently gave a presentation to our local ColdFusion group titled 'How NOT to Code in ColdFusion' where I showed simple mistakes that I run across. I figured this might make an interesting series of posts on my blog as well. So here's the first example. The game is to find as many mistakes with the code as possible, and yes, there will always be several to pull out of them. Naturally you may want to see how many you can find before scrolling down for the comments where people give their answers. As far as style type issues I'm not as concern with them per se, but you're free to list them if you like. I'll probably bounce around with languages like AS3, Flex, CFML and AJAX - so hopefully people will find this fun.
<cfquery datasource="#sysdb#" name="adduser">
insert into users(firstname,mi,lastname,email)
values('#firstname#','#mi#','#lastname#','#email#');
</cfquery>
<cfquery datasource="#sysdb#" name="id">
select max(id) as newid from users where 0=0
</cfquery>
<cfset session.id = id.newid/>
Nathan Strutz wrote on 01/10/104:21 PM
Fun! A game!Let's see. First query doesn't need a name attribute. Values need cfqueryparam tags instead of raw outputs. Semicolon is unnecessary and not consistent with the second query.
Depending on your database, the 2nd query could be put inside the same cfquery block, reducing the number of round trips (but then putting the need for the name attribute back in). Selecting the max id is a crappy way to get the ID of the row of data that was just inserted - there are typically database specific ways to do this that will not choke on race conditions such as SQLServer's SCOPE_IDENTITY(), or in Oracle where you need to create the identity before you insert the record.
If you're going to have 2 cfqueries that are related, they should be nested in a cftransaction. To avoid race conditions with this exact code, you may also need to put on a cflock around the entire block. Neither of these would be necessary if they were combined into one cfquery statement.
Hmm... think I missed anything?