better way than lots of if's

[coolicus]

I have a script that checks before and after values in a DB then emails the user with the bits that have changed. I created variables from the DB first as strStep1 etc:

if request.form("step1") <> strStep1 then
alertStep1 = true
end if
if request.form("step2") <> strStep2 then
alertStep2 = true
end if
if request.form("step3") <> strStep3 then
alertStep3 = true
end if
if request.form("step4") <> strStep4 then
alertStep4 = true
end if
if request.form("step5") <> strStep5 then
alertStep5 = true
end if
if request.form("step6") <> strStep6 then
alertStep6 = true
end if
if request.form("step7") <> strStep7 then
alertStep7 = true
end if
if request.form("step8") <> strStep8 then
alertStep8 = true
end if
if request.form("step9") <> strStep9 then
alertStep9 = true
end if

then when I send the user an email to tell them of their change

if alertStep1 then
response.write "Step 1 has been updated, you now need to ...."
end if
if alertStep2 then
response.write "Step 2 has been updated, you now need to ...."
end if
etc etc etc

Obviously only one thing may have changed one time, another time three things may have changed. Is there a better way to do this or is my code ok? My code works fine just wondered if I should be doing it another way.

 

[onpnt]

You have consistent naming in your variables and in the collection so you can loop the form collection while testing the condition on the variable. Note you will need to utilize the Execute function to dynamically refer to a variable name

[sub]____________ signature below ______________
You are a amateur developer until you realize all your code sucks.
Jeff Atwood[/sub]

 

[monksnake]

The only thing that's really a pain about multiple ifs is that is can be a lot to type. Typically though, it runs very fast.

The only thing I can see to possibly optimize this code is to combine the 2 snippets of code you have.

//Make a variable to contain the email body
Dim emailBody
Set emailBody = "";
if request.form("step1") <> strStep1 then
emailBody += "Step 1 has been updated, you now need to ...."
end if
if request.form("step2") <> strStep2 then
emailBody += "Step 2 has been updated, you now need to ...."
end if
if request.form("step3") <> strStep3 then
emailBody += "Step 3 has been updated, you now need to ...."
end if
if request.form("step4") <> strStep4 then
emailBody += "Step 4 has been updated, you now need to ...."
end if
......

//Later in Email area
Response.Write(emailBody)

<.

 

[onpnt]

Bit a few different languages there monk ;-)

[sub]____________ signature below ______________
You are a amateur developer until you realize all your code sucks.
Jeff Atwood[/sub]

 

[monksnake]

Damn VBScript and their lack of += !!!



<.

 

[onpnt]

I know it. That bothered me for years

[sub]____________ signature below ______________
You are a amateur developer until you realize all your code sucks.
Jeff Atwood[/sub]

 

[Hans8823]

For i = 1 To 9
If Request.Form("step" & i) <> strStep(i) Then
  alertstep(i) = True
End If

For i = 1 To 9
If alertstep(i) Then
  response.write "Step " & i & " has been updated, you now need to ...."
End If

 

[onpnt]

Hans8823 that needs more explanation or it will only cause the OP more headaches. You think?



[sub]____________ signature below ______________
You are a amateur developer until you realize all your code sucks.
Jeff Atwood[/sub]

 

[Hans8823]

What explanation do you want? Since every if statement is the same, only the number is different, so I thought why not use arrays?

 

[Sheco]

Hans8823, why not just dump the array and use a single For loop with the Response.Write inside?

Also the Next is missing.

 

[coolicus]

Thanks guys, I got it.

I couldn`t use the loop Hans suggested as some of the variables are not of the format strStep(i) but strAddress etc.

I used monks suggestion, I was adding the extra process with the booleans, oh and I didn`t use the += hehe

 

[coolicus]

The dates are causing a problem for me.

I have this part

if request.form("step2target") <> strStep2target then
changeditems = changeditems & "Step 2 has been updated.<br />"
end if

Both request.form("step2target") and strStep2target are the same as I did not change them in my edit form however this line is being added to my output email.

I used this to check the outputs of them both

response.write "from form:" & request.form("step2target")
response.write "<br />from DB: " & strStep2target
response.end

and the output was

from form:05/06/2007
from DB:05/06/2007

so they are the same. This only happens with fields of data type datetime in my MSSQL database. All varchar or text fields work fine.

Should I change all of the datetime fields to varchar in the DB? Are there any issues with holding a datetime in a varchar field?

Thanks again

 

[monksnake]

You can do two things here without changing you DB structure: Convert your datetime fields to varchar when you pull the data in your query or parse your form date values into dates.

You should keep the datetime fields as datetime in your DB.

<.

 

[coolicus]

Thanks monk, do I convert the fields to varchar in asp or in my SQL command?

I have googled but didn`t find owt.

Is this a common problem with datetime fields?

 

[monksnake]

I'd do it in your query (SQL command).

I've ran into it a few times, ASP grabs the date from the database and puts it into it's own date format.

I think it's kinda common.

There was a question a few days ago about the same thing,

here is that post:

thread333-1372893

<.

 

[bslintx]

coolicus,

it's understood you may be content with the else if ladder up to this point but per original post...you basically asked if it could be more efficient/tidey...

you also mentioned you couldn't use hans's attempt because of "other" data types...

it seems like there is a lot of unnecessary repetition going on...perhaps the combination of onpnt's suggestion, if else logic, and/or sub/function could help?

if satisfied then please disregard...else, perhaps lay ALL your cards down for us to see...so we can get a better understanding of how we can help you possibly make it work...

btw, to be fair to hans's suggestion (minus sheco's catch and sheco's suggestion)...it seemed pretty logical at what he was trying to do...considering what was originally posted

good luck!

 

[kss444]

You could have used a case statment instead of the if's.

Ordinary Programmer

 

[ESquared]

I have a hunch that there may be an even better way to help you. Would you tell us what you're doing in the page and give us some examples of code, input, and output of each page?

Cum catapultae proscriptae erunt tum soli proscript catapultas habebunt.