I agree completely. String substitution in SQL is pretty stupid (it's not very good at string manipulation), non-parameterized SQL queries are very stupid as they're easily subjected to SQL Injection attacks, and both together (and there's a third language here as well, HTML) is just confusing as all hell. I'd split this up into only retrieving actual values from SQL Server using parameterized queries, writing a real JavaScript function that calls window.open, then writing formatted string code to generate the HTML to call the JavaScript function passing the parameters, and passing that through an HTML/XML entity encoding routine where that turns out to be necessary. As it is, that's an unmaintainable mess. DoEvents: Generating unexpected recursion since 1991

I had the same objection to this while going through some code with the boss many years ago. I always checked for null separately before doing anything else. He did it just the same way like in the code here. He argued that the check for null values is always done first and does not run into the second term if the value is indeed null. This may not be sound proof, but the entire code was full of this and we never had any problems because of that. A while ago he asked me what he should have printed on my business cards. I said 'Wizard'. I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.