Help with for loop
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
I will check quickly.
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
It seems its initial value is 0RuadRauFlessa wrote:What is the value of txtNumRisks.Text when you get to lstHazards.Items.Clear()
By the time it hits lstHazards it seems to be 1.
Last edited by CesarePlay on 19 Jan 2009, 16:49, edited 1 time in total.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
Whoooot
After all of
txtNumImpacts.Text is "0"
Now that does not make sence.
Also is
and
Supposed to be the same?
After all of
Code: Select all
For counter = 0 To (ImpArray.Length - 1)
If IsNumeric(ImpArray(counter)) Then
'GET NUMBER OF RISKS
Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
Dim dsNumRisks As New DataSet
dsNumRisks = currCC.executeQuery(Application("ConnString"), strSQL1, "PPE")
'Create Data Row
Dim rNumRisks As DataRow
For Each rNumRisks In dsNumRisks.Tables("PPE").Rows
txtNumRisks.Text = CInt(txtNumRisks.Text) + 1
Next
If txtNumImpacts.Text = "" Then
txtNumImpacts.Text = "1"
Else
txtNumImpacts.Text = CInt(txtNumImpacts.Text) + 1
End If
End If
Next
Now that does not make sence.
Also is
Code: Select all
'GET NUMBER OF RISKS
Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
Code: Select all
'GET RISKS
Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "'"
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
It seems by that time it is 1. I have another reference to it further down after I said it was 0
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
That I will check quick.RuadRauFlessa wrote:
Also is
andCode: Select all
'GET NUMBER OF RISKS Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _ "FROM _SAFETY_Risks INNER JOIN " & _ "_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _ "WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
Supposed to be the same?Code: Select all
'GET RISKS Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _ "FROM _SAFETY_Risks INNER JOIN " & _ "_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _ "WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "'"
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
One has the array in the sql string. That does not affect the data passing through on either statement.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
Try something like
And check what the value of txtNumRisks.Text is. It should not make a diff but yeah.
Code: Select all
For counter = 0 To (ImpArray.Length - 1)
If IsNumeric(ImpArray(counter)) Then
'GET NUMBER OF RISKS
Dim strSQL1 = "SELECT count(*) " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
'Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
' "FROM _SAFETY_Risks INNER JOIN " & _
' "_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
' "WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
Dim dsNumRisks As New DataSet
dsNumRisks = currCC.executeQuery(Application("ConnString"), strSQL1, "PPE")
'Create Data Row
Dim rNumRisks As DataRow
For Each rNumRisks In dsNumRisks.Tables("PPE").Rows
txtNumRisks.Text = 'retrieve col 1 from rNumRisks
'txtNumRisks.Text = CInt(txtNumRisks.Text) + 1
Next
If txtNumImpacts.Text = "" Then
txtNumImpacts.Text = "1"
Else
txtNumImpacts.Text = CInt(txtNumImpacts.Text) + 1
End If
End If
Next
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
Ok. I will try that.
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
I solved half the problem but now it is duplicating in another table that comes from this code. I fixed the table risk one. Now the hazards is looking at other code and using the info and assigning it all the other risk types.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
I post it now.
This solved risk error but not another duplicate error from been done in another table that is referenced from this code under Hazards & risks.
Code: Select all
Dim counter As Integer
counter = 0
Dim RiskCounter As Integer
Dim OccCounter As Integer
Dim EquipCounter As Integer
RiskCounter = 0
OccCounter = 0
EquipCounter = 0
Dim ImpArray As Array
ImpArray = ImpactsArray("IMPACTS", r)
For counter = 0 To (ImpArray.Length - 1)
If IsNumeric(ImpArray(counter)) Then
'GET NUMBER OF RISKS
Dim strSQL = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "' AND _EMS_Reg_Impacts.ID = '" & ImpArray(counter) & "'"
Dim dsNumRisks As New DataSet
dsNumRisks = currCC.executeQuery(Application("ConnString"), strSQL, "NumRisks")
'Create Data Row
Dim rNumRisks As DataRow
For Each rNumRisks In dsNumRisks.Tables("NumRisks").Rows
txtNumRisks.Text = CInt(txtNumRisks.Text) + 1
Next
If txtNumImpacts.Text = "" Then
txtNumImpacts.Text = "1"
Else
txtNumImpacts.Text = CInt(txtNumImpacts.Text) + 1
End If
End If
Next
lstHazards.Items.Clear()
Dim initItem As New System.Web.UI.WebControls.ListItem
initItem.Value = "0"
initItem.Text = "-- Not Selected --"
lstHazards.Items.Add(initItem)
For counter = 0 To (ImpArray.Length - 1)
If IsNumeric(ImpArray(counter)) Then
'Response.Write("hello" & "<br>")
'==============Get Impact Name==================
strSQL = "SELECT * FROM _EMS_Reg_Impacts WHERE ID = '" & ImpArray(counter) & "'"
Dim dsImpName As New DataSet
dsImpName = currCC.executeQuery(Application("ConnString"), strSQL, "ImpName")
Dim rImpName As DataRow
If dsImpName.Tables("ImpName").Rows.Count > 0 Then
Response.Write("hello" & "<br>")
rImpName = dsImpName.Tables("ImpName").Rows(0)
'============HAZARDS DROP-DOWN=============
Dim currItem As New System.Web.UI.WebControls.ListItem
currItem.Value = ImpArray(counter)
currItem.Text = rImpName("IMPACT")
lstHazards.Items.Add(currItem)
'=========================================
End If
'End If
End if
Next
'GET RISKS
Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = '" & lstItems.SelectedItem.Value & "'"
'Response.Write(strSQL1 & "<br>")
Dim ds2 As New DataSet
ds2 = currCC.executeQuery(Application("ConnString"), strSQL1, "Risks")
'Create Data Row
Dim r2 As DataRow
For Each r2 In ds2.Tables("Risks").Rows
Dim cell1 As New System.Web.UI.WebControls.TableCell
Dim cell2 As New System.Web.UI.WebControls.TableCell
Dim cellremove As New System.Web.UI.WebControls.TableCell
Dim tr As New System.Web.UI.WebControls.TableRow
cell1.Text = currCC.retrieveString("RISK", r2)
cell2.Text = currCC.retrieveString("HAZARDDESC", r2)
cell1.CssClass = "content3"
cell2.CssClass = "content3"
cellremove.CssClass = "content3"
tr.Cells.Add(cell1)
tr.Cells.Add(cell2)
Dim cmdRemove As New System.Web.UI.HtmlControls.HtmlButton
cmdRemove.Attributes.Add("onclick", "removerisks('" & r2("ID") & "')")
cmdRemove.InnerText = "Remove"
cmdRemove.Style.Add("font-size", "10px")
cellremove.Controls.Add(cmdRemove)
tr.Cells.Add(cellremove)
tblRisks.Rows.Add(tr)
New code
For counter = 0 To (ImpArray.Length - 1)
If IsNumeric(ImpArray(counter)) Then
makeImpactRow(ImpArray(counter), RiskCounter, r2)
RiskCounter += 1
End If
Next
end of new code
Next
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
Ah I see what you have done. You took the risk part out of the for loop. So if I understand you correctly the risk stuff is sorted but you still sit with duplicates under hazards ?
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
Yes. Thats the new problem. Solving risks led to error in hazards.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
What you should do is code a function that getRisks and getHazards and pass the selction to them and return a list of risks or hazards from them. Then you would eliminate the whole problem. I would actually vote for a rewrite of this code as you are always going to have a problem with it. It is badly written and as you can see very buggy. There is almost no abstraction and it would seem that everything is just done in one function which is not the way to do it. We live in the world of objects, threading and events not in a world of linear code and single proccesses.
If I was you I would rather take 3 to 5 hours to properly rewrite the whole piece of code rather than sit one more minit trying to sort out the bugs in this. Also I think you can do a better job of it than the previous guy.
If I was you I would rather take 3 to 5 hours to properly rewrite the whole piece of code rather than sit one more minit trying to sort out the bugs in this. Also I think you can do a better job of it than the previous guy.
- Spoiler (show)
- Ron2K
- Forum Technical Administrator
- Posts: 9050
- Joined: 04 Jul 2006, 16:45
- Location: Upper Hutt, New Zealand
- Contact:
Re: Help with for loop
This isn't related to the problem you're currently experiencing, it's a general coding tip for the future - assuming you're using ASP .NET and not classic ASP (if you're using classic ASP, then stop reading this post right now).
I notice you have stuff in your code like this:
I would never insert any variable directly into a SQL statement like that, as you run the risk of SQL injection.
There are two methods of dealing with it. The first is to carry on like you've been doing, but search all variables going into the statement for characters that can cause SQL injection, and escape them properly. I don't like doing this though, I prefer using parameters in my queries. It's a bit longer to do, but it's more elegant and it makes you safe from SQL injection (I have tested this extensively).
Here's a code sample, written in C#, that illustrates this:
I notice you have stuff in your code like this:
Code: Select all
'GET RISKS
Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _
"FROM _SAFETY_Risks INNER JOIN " & _
"_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _
"WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = " & lstItems.SelectedItem.Value & "'"
There are two methods of dealing with it. The first is to carry on like you've been doing, but search all variables going into the statement for characters that can cause SQL injection, and escape them properly. I don't like doing this though, I prefer using parameters in my queries. It's a bit longer to do, but it's more elegant and it makes you safe from SQL injection (I have tested this extensively).
Here's a code sample, written in C#, that illustrates this:
Code: Select all
string sSQL = @"
SELECT * FROM tblCustomers WITH (nolock)
WHERE customerID = @CustomerID";
SqlCommand sqlCommand = new SqlCommand(sSQL, sqlConn); // sqlConn is of type SqlConnection
sqlCommand.Parameters.Add("@CustomerID",
SqlDbType.Int).Value = 12345; // in real life, this will be a variable
SqlDataReader reader = sqlCommand.ExecuteReader();
if (reader.HasRows)
{
// code here to process the rows
}
if (!reader.IsClosed) reader.Close();
Kia kaha, Kia māia, Kia manawanui.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
Nice example there R2K. It is a very valid point you are making.
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
I solved it now finally.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
Hi. I understand what you saying. However I did not write this code. This code was done in 1995 and been upgraded since then. I will try implement what you are saying into this application as I fix it.Ron2K wrote:This isn't related to the problem you're currently experiencing, it's a general coding tip for the future - assuming you're using ASP .NET and not classic ASP (if you're using classic ASP, then stop reading this post right now).
I notice you have stuff in your code like this:
I would never insert any variable directly into a SQL statement like that, as you run the risk of SQL injection.Code: Select all
'GET RISKS Dim strSQL1 = "SELECT _SAFETY_Risks.*, _EMS_Reg_Impacts.IMPACT AS HAZARDDESC " & _ "FROM _SAFETY_Risks INNER JOIN " & _ "_EMS_Reg_Impacts ON _SAFETY_Risks.HAZARD = _EMS_Reg_Impacts.ID " & _ "WHERE (_SAFETY_Risks.REC_COUNT = 0) AND (_EMS_Reg_Impacts.REC_COUNT = 0) AND ASPECTID = " & lstItems.SelectedItem.Value & "'"
There are two methods of dealing with it. The first is to carry on like you've been doing, but search all variables going into the statement for characters that can cause SQL injection, and escape them properly. I don't like doing this though, I prefer using parameters in my queries. It's a bit longer to do, but it's more elegant and it makes you safe from SQL injection (I have tested this extensively).
Here's a code sample, written in C#, that illustrates this:
Code: Select all
string sSQL = @" SELECT * FROM tblCustomers WITH (nolock) WHERE customerID = @CustomerID"; SqlCommand sqlCommand = new SqlCommand(sSQL, sqlConn); // sqlConn is of type SqlConnection sqlCommand.Parameters.Add("@CustomerID", SqlDbType.Int).Value = 12345; // in real life, this will be a variable SqlDataReader reader = sqlCommand.ExecuteReader(); if (reader.HasRows) { // code here to process the rows } if (!reader.IsClosed) reader.Close();
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
RuadRauFlessa wrote:What was the problem?
It was calling the wrong function. Here is the fixed code
Code: Select all
makeImpactRow(r2("Hazard"), RiskCounter, r2)
RiskCounter += 1
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
I thought as much.CesarePlay wrote:RuadRauFlessa wrote:What was the problem?
It was calling the wrong function. Here is the fixed code
Code: Select all
makeImpactRow(r2("Hazard"), RiskCounter, r2) RiskCounter += 1
Best idea ever.CesarePlay wrote: This works. I try and get this page to be more standards correct when I have time.
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
It will be hard to do it but some of it has been changed to be better. It also has been better organized now. Hopefully it won't take long to do it.RuadRauFlessa wrote: Best idea ever.
- Ron2K
- Forum Technical Administrator
- Posts: 9050
- Joined: 04 Jul 2006, 16:45
- Location: Upper Hutt, New Zealand
- Contact:
Re: Help with for loop
Problem is, the real world is often messy - there will be times where you'll have to fix something you can't rewrite, particularly when you're dealing with legacy stuff.
Kia kaha, Kia māia, Kia manawanui.
-
- Registered User
- Posts: 20576
- Joined: 19 Sep 2003, 02:00
- Location: Bloodbank
Re: Help with for loop
That is true Ron2K. I just prefer having it properly done even if it means that I have to sit into the night rewriting something which someone else did 10 years ago.
- Spoiler (show)
-
- Registered User
- Posts: 10628
- Joined: 26 Mar 2007, 02:00
- Location: In the river of thoughts
- Contact:
Re: Help with for loop
Thats true. If I have to add new stuff I make sure the new stuff is done correctly to it.Ron2K wrote: Problem is, the real world is often messy - there will be times where you'll have to fix something you can't rewrite, particularly when you're dealing with legacy stuff.