Looping Code, Save time

[stephen.s13]

Hi all.
So here the deal.
I I making a program to monitor the connection state to 54 servers.
The code works perfect but it's rediculously long and if I want to alter it at any point. I have to change it 54 times!

Can someone assist me in looping the script so it does one block of code 54 times (or 55 if we get another server etc)

here are the first two blocks

        Label1.Text = "Checking Store 01: 1st attempt (1000ms)"
        ProgressBar1.Value = 1
        If My.Computer.Network.Ping("192.168.1.1", 1000) Then
            If p01.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 01, connection restablished" & Environment.NewLine)
            p01.BackColor = Color.Lime
            t01.BackColor = Color.Lime
        Else
            Label1.Text = "Checking Store 1: 2nd attempt (5000ms)"
            If My.Computer.Network.Ping("192.168.1.1", 5000) Then
                If p01.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 01, connection restablished" & Environment.NewLine)
                p01.BackColor = Color.Lime
                t01.BackColor = Color.Lime
            Else
                Label1.Text = "Checking Store 1: 3rd attempt (10000ms)"
                If My.Computer.Network.Ping("192.168.1.1", 10000) Then
                    If p01.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 01, connection restablished" & Environment.NewLine)
                    p01.BackColor = Color.Lime
                    t01.BackColor = Color.Lime
                Else
                    If p01.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store 01, connection dropped" & Environment.NewLine)
                    p01.BackColor = Color.Red
                    t01.BackColor = Color.Red
                End If
            End If
        End If

        '****************************************************************************************
        Label1.Text = "Checking Store 02: 1st attempt (1000ms)"
        ProgressBar1.Value = 2
        If My.Computer.Network.Ping("192.168.2.1", 1000) Then
            If p02.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 02, connection restablished" & Environment.NewLine)
            p02.BackColor = Color.Lime
            t02.BackColor = Color.Lime
        Else
            Label1.Text = "Checking Store 2: 2nd attempt (5000ms)"
            If My.Computer.Network.Ping("192.168.2.1", 5000) Then
                If p02.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 02, connection restablished" & Environment.NewLine)
                p02.BackColor = Color.Lime
                t02.BackColor = Color.Lime
            Else
                Label1.Text = "Checking Store 2: 3rd attempt (10000ms)"
                If My.Computer.Network.Ping("192.168.2.1", 10000) Then
                    If p02.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store 02, connection restablished" & Environment.NewLine)
                    p02.BackColor = Color.Lime
                    t02.BackColor = Color.Lime
                Else
                    If p02.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store 02, connection dropped" & Environment.NewLine)
                    p02.BackColor = Color.Red
                    t02.BackColor = Color.Red

                End If
            End If
        End If
        '****************************************************************************************

[stephen.s13]

Trying but I i'm having no joy lol

For value As Integer = 1 To 54
            Dim pb As PictureBox = "p" And value
            Dim tb As TextBox = "t" And Value

[stephen.s13]

I am getting the error that type long cannot be converted....

For value As Integer = 1 To 54
            Dim pb As PictureBox = "p" And value
            Dim tb As TextBox = "t" And value

            Label1.Text = "Checking Store " & value & ": 1st attempt (1000ms)"
            ProgressBar1.Value = value
            If My.Computer.Network.Ping("192.168." & value & ".1", 1000) Then
                If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                pb.BackColor = Color.Lime
                tb.BackColor = Color.Lime
            Else
                Label1.Text = "Checking Store 1: 2nd attempt (5000ms)"
                If My.Computer.Network.Ping("192.168." And value And ".1", 5000) Then
                    If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                    pb.BackColor = Color.Lime
                    tb.BackColor = Color.Lime
                Else
                    Label1.Text = "Checking Store " And value And ": 3rd attempt (10000ms)"
                    If My.Computer.Network.Ping("192.168." And value And ".1", 10000) Then
                        If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                        pb.BackColor = Color.Lime
                        tb.BackColor = Color.Lime
                    Else
                        If pb.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store " And value And ", connection dropped" & Environment.NewLine)
                        pb.BackColor = Color.Red
                        tb.BackColor = Color.Red
                    End If
                End If
            End If
        Next

[techgnome]

A few things...
1) you're name is stephen, right? and you are a human, right (well, you seem human, I suppose you could be anything, we do have a gnome, a dog, a three-eyed frog and a vase of fish here, so maybe we should go with humanoid)... so I couldn't point to you and call you a bicycle now can I? Because you're not a bicycle. But that's what you did with these lines:
Dim pb As PictureBox = "p" And value
Dim tb As TextBox = "t" And value

you declared a bicycle (Dim pb as PictureBox) and then called it stephen ("p" and Value) ... "p" is a string... so it cannot be a picture box... the fact that it happens to be part of the name of a pic box is irrelevant. The Good news is that every form (and every container for that matter) has a .Controls collection... which does contain all of the controls (directly) on it ... more on that in a moment as there's another issue to sort out.

2) "p" And value -- doesn't do what I think you think it should do. And is a boolean/bit operator... it looks at the bits of the two values, turns them on when there is a match on both sides and off when there's a mismatch... it does not concatenate them together. & is the preferred operand for that. You got that right in a few places through out the code, and used "and" in others...
Back to the dim statements:
Dim pb As PictureBox = Me.Controls("p" & value.ToString)
Dim tb As TextBox = Me.Controls("t" & value.ToString)

NOW we have strings where we need them, properly concatenated, and used to pass to the Control property of the form which will then return the correct PictureBox and Text box.

Note - this wasn't the only place where you used "and" instead of "&"... so you'll need to go through the rest of the code.

WARNING - this will work if the pictureboxes and textboxes are on the form DIRECTLY, if they are contained in a group box, or another container... it may take a bit more finesse to get to them, but it does make a difference.

-tg

[digitalShaman]

very good and amusing answer from tg. only problem i spotted was: "it looks at the bits of the two values, turns them on when there is a match on both sides" that would be XNOR which i am not aware its implemented

so you eliminated a whole bunch of redundant code with that change. great. now some more art of these:

step 2:

		For value As Integer = 1 To 54
            Dim pb As PictureBox = "p" And value
            Dim tb As TextBox = "t" And value

            Label1.Text = "Checking Store " & value & ": 1st attempt (1000ms)"
            ProgressBar1.Value = value
            dim isAlive as boolean=false
            If My.Computer.Network.Ping("192.168." & value & ".1", 1000) Then
            	isalive=true
            Else
                Label1.Text = "Checking Store 1: 2nd attempt (5000ms)"
                If My.Computer.Network.Ping("192.168." And value And ".1", 5000) Then
	            	isalive=true
                Else
                    Label1.Text = "Checking Store " And value And ": 3rd attempt (10000ms)"
                    If My.Computer.Network.Ping("192.168." And value And ".1", 10000) Then
		            	isalive=true
                    End If
                End If
            End If
            
            if isalive then
                    If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                    pb.BackColor = Color.Lime
                    tb.BackColor = Color.Lime
            else
                    If pb.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store " And value And ", connection dropped" & Environment.NewLine)
                    pb.BackColor = Color.Red
                    tb.BackColor = Color.Red
            end if
        Next

now you realize that you are concating the ip multiple times:

		For value As Integer = 1 To 54
            Dim pb As PictureBox = "p" And value
            Dim tb As TextBox = "t" And value

            Label1.Text = "Checking Store " & value & ": 1st attempt (1000ms)"
            ProgressBar1.Value = value
            dim isAlive as boolean=false
            dim sIP as string="192.168." & value & ".1"
            If My.Computer.Network.Ping(sIP, 1000) Then
            	isalive=true
            Else
                Label1.Text = "Checking Store 1: 2nd attempt (5000ms)"
                If My.Computer.Network.Ping(sIP, 5000) Then
	            	isalive=true
                Else
                    Label1.Text = "Checking Store " And value And ": 3rd attempt (10000ms)"
                    If My.Computer.Network.Ping(sIP, 10000) Then
		            	isalive=true
                    End If
                End If
            End If
            
            if isalive then
                    If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                    pb.BackColor = Color.Lime
                    tb.BackColor = Color.Lime
            else
                    If pb.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store " And value And ", connection dropped" & Environment.NewLine)
                    pb.BackColor = Color.Red
                    tb.BackColor = Color.Red
            end if
        Next

ok but three pings, just because different timeouts?

		For value As Integer = 1 To 54
            Dim pb As PictureBox = "p" And value
            Dim tb As TextBox = "t" And value

            ProgressBar1.Value = value
            dim isAlive as boolean=false
            dim sIP as string="192.168." & value & ".1"

            dim msTimeout as int32=1000
            do
            	Label1.Text = string.format("Checking Store {0}: attempt #{1} ({2}ms)",value,(mstimeout\5000)+1,msTimeout)
            	isAlive = My.Computer.Network.Ping(sIP, msTimeout)
            	msTimeout = if(msTimeout=1000, 5000,10000)
            loop until isAlive or msTimeout=10000
            
            if isalive then
                    If pb.BackColor = Color.Red Then Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
                    pb.BackColor = Color.Lime
                    tb.BackColor = Color.Lime
            else
                    If pb.BackColor = Color.Lime Then Log.AppendText(Now.ToString & " :Store " And value And ", connection dropped" & Environment.NewLine)
                    pb.BackColor = Color.Red
                    tb.BackColor = Color.Red
            end if
        Next

i must admit, the # attempts for the label.text entry are a bit cryptic. i'd consider droping this and just say "checking store x, yyyy timeout"

ok but you then realize that you do not need the pb before the end of the pings so cleaning up a bit:

		For value As Integer = 1 To 54
            ProgressBar1.Value = value

            dim isAlive as boolean=false
            dim sIP as string="192.168." & value & ".1"
            dim msTimeout as int32=1000
            do
            	Label1.Text = string.format("Checking Store {0}: attempt #{1} ({2}ms)",value,(mstimeout\5000)+1,msTimeout)
            	isAlive = My.Computer.Network.Ping(sIP, msTimeout)
            	msTimeout = if(msTimeout=1000, 5000,10000)
            loop until isAlive or msTimeout=10000
            
            
            Dim pb As PictureBox = me.controls("p" & value)
            Dim tb As TextBox = me.controls("t" & value)
            if isalive andalso pb.BackColor = Color.Red Then
            	Log.AppendText(Now.ToString & " :Store " And value And ", connection restablished" & Environment.NewLine)
            elseif not isAlive andalso pb.BackColor = Color.Lime Then
            	Log.AppendText(Now.ToString & " :Store " And value And ", connection dropped" & Environment.NewLine)
            end if
            pb.BackColor=if(isAlive,Color.Lime,Color.Red)
            tb.BackColor=pb.BackColor
        Next

all untested, just typed in here.

[techgnome]

very good and amusing answer from tg. only problem i spotted was: "it looks at the bits of the two values, turns them on when there is a match on both sides" that would be XNOR which i am not aware its implemented

That is correct... I misspoke... it should have read "... turns them one when both sides are true, and turns them off when not..."

-tg

[dday9]

The way that I would go about doing this to setup a method that accepts a string collection as a parameter to represent all of the IP addresses:

Private Sub CheckStore(ByVal ipAddresses() As String)

End Sub

Then inside of the method, use a For/Each loop to loop through the collection:

For Each ip As String in ipAddresses

Next

Assuming that each store has a unique IP address(which it probably does) then you could use IndexOf to get the value of your value variable:

Dim value As Integer = Array.IndexOf(ipAddresses, ip) + 1 'Add 1 for 0 based index
Label1.Text = "Checking Store " & value.ToString() & ": 1st attempt (1000ms)"

Then from there it is just a matter of executing your code.