Help with for loop

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 16:43

I will check quickly.

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 16:44

RuadRauFlessa wrote:What is the value of txtNumRisks.Text when you get to lstHazards.Items.Clear()

It seems its initial value is 0

By the time it hits lstHazards it seems to be 1.

RuadRauFlessa · Post by **RuadRauFlessa** » 19 Jan 2009, 16:48

Whoooot

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

txtNumImpacts.Text is "0"

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) & "'"

and

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 & "'"

Supposed to be the same?

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 16:50

It seems by that time it is 1. I have another reference to it further down after I said it was 0

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 16:51

RuadRauFlessa wrote:
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) & "'"

and

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 & "'"

Supposed to be the same?

That I will check quick.

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 16:59

One has the array in the sql string. That does not affect the data passing through on either statement.

RuadRauFlessa · Post by **RuadRauFlessa** » 19 Jan 2009, 17:08

Try something like

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

And check what the value of txtNumRisks.Text is. It should not make a diff but yeah.

CesarePlay · Post by **CesarePlay** » 19 Jan 2009, 17:10

Ok. I will try that.

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 09:43

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.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 09:47

So how does the code look now

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 09:53

I post it now.

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

This solved risk error but not another duplicate error from been done in another table that is referenced from this code under Hazards & risks.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 09:59

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 ?

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 10:06

Yes. Thats the new problem. Solving risks led to error in hazards.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 10:13

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.

Post by **Ron2K** » 20 Jan 2009, 10:44

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:

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 & "'"

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:

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();

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 10:49

Nice example there R2K. It is a very valid point you are making.

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 10:57

I solved it now finally.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 10:58

What was the problem?

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 11:06

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:
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 & "'"
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:
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();

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.

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 11:09

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

This works. I try and get this page to be more standards correct when I have time.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 11:11

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

I thought as much.

CesarePlay wrote: This works. I try and get this page to be more standards correct when I have time.

Best idea ever.

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 11:14

RuadRauFlessa wrote: Best idea ever.

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.

Post by **Ron2K** » 20 Jan 2009, 11:45

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.

RuadRauFlessa · Post by **RuadRauFlessa** » 20 Jan 2009, 11:53

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.

CesarePlay · Post by **CesarePlay** » 20 Jan 2009, 11:57

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.

Thats true. If I have to add new stuff I make sure the new stuff is done correctly to it.

PCFormat Forum Archive

Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop

Re: Help with for loop