?
Solved

VB.NET -> Cutting down on If statements

Posted on 2014-02-10
4
Medium Priority
?
314 Views
Last Modified: 2014-02-28
This is more of a learning exercise as the code works but i wanted to know if i could have done it with less code or better logic

Function Getthescores(ByVal whatwebpage As String, ByVal whattoloop As String, ByVal hometeam As String, ByVal awayteam As String, ByVal setthetime As String, ByVal homescore As String, ByVal awayscore As String) As String
        Dim thescorestext As String
        Dim webGet As New HtmlWeb() 'open the system
        Dim htmlDoc As HtmlDocument = webGet.Load(whatwebpage) '' get the html from the webpage

        Dim coll As HtmlNodeCollection = htmlDoc.DocumentNode.SelectNodes(whattoloop)
        ' do stuff
        If coll IsNot Nothing Then

            For Each div As Object In coll ' select all the divs within the code that contain *

                If Not div.selectSingleNode(hometeam) Is Nothing Then
                    'set the time to make sure it can be orderd

                    Dim settime As String = System.Text.RegularExpressions.Regex.Replace(div.selectSingleNode(setthetime).InnerText, "[^a-zA-Z0-9]", " ")

                    If settime <> "" Then
                        settime = settime.Substring(1, 2)
                    End If

                    Dim orderbytime As Integer
                    If settime = "HT" Then
                        orderbytime = 45
                    ElseIf settime = "FT" Then
                        orderbytime = 999
                    Else
                        orderbytime = Convert.ToInt32(settime)
                    End If
                            thescorestext = ""

                    thescorestext += StringOutNumbers(div.selectSingleNode(hometeam).InnerText) 'get home team name
                  
                    thescorestext += div.selectSingleNode(homescore).InnerText & " - " & div.selectSingleNode(awayscore).InnerText

                    thescorestext += StringOutNumbers(div.selectSingleNode(awayteam).InnerText) ' get away teams name

                    Dim count = scorestable.Rows.Cast(Of DataRow)().Where(Function(n) n("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(hometeam).InnerText)).Count()
                    'Find the count
                    If count = 0 Then
                        'if not finished add row to the datatable as no results where found

                        scorestable.Rows.Add(StringOutNumbers(div.selectSingleNode(hometeam).InnerText), settime, thescorestext, "", "", orderbytime)
                    Else
                        'A matching result was found so now check to see if the Scores section of the result matches the value - thescorestext
                        For Each row As DataRow In scorestable.Rows
                            If row("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(hometeam).InnerText) And row("Score").ToString() <> thescorestext Then
                                'add scores
                                row.SetField("Score", thescorestext)
                                row.SetField("Time", settime)
                                row.SetField("Lastgoaltime", DateTime.Now())
                                row.SetField("Kickoff", orderbytime)
                            ElseIf row("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(hometeam).InnerText) And row("Score").ToString() = thescorestext Then
                                ' add time
                                row.SetField("Time", settime)
                                row.SetField("Kickoff", orderbytime)
                            End If
                        Next
                    End If
                End If
            Next
        End If
        Return thescorestext


    End Function

Open in new window

0
Comment
Question by:runnerjp2005
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
4 Comments
 
LVL 143

Expert Comment

by:Guy Hengel [angelIII / a3]
ID: 39846981
no, I don't see any way to "improve" this, except by adding comments  ...
Dim coll As HtmlNodeCollection = htmlDoc.DocumentNode.SelectNodes(".//li[@class='avb-row IN_PLAY']")
		' do stuff
If coll IsNot Nothing Then

        For Each div As Object In htmlDoc.DocumentNode.SelectNodes(".//li[@class='avb-row IN_PLAY']") ' select all the divs within the code that contain *

         If Not div.selectSingleNode(".//span[@class='home-team-name']") Is Nothing Then
                'set the time to make sure it can be orderd



                Dim settime As String = System.Text.RegularExpressions.Regex.Replace(div.selectSingleNode(".//span[@class='event-inplay-state inplay ui-time-stop-format ']").InnerText, "[^a-zA-Z0-9]", " ")

                If settime <> "" Then
                    settime = settime.Substring(1, 2)
                End If

                Dim orderbytime As Integer
                If settime = "HT" Then
                    orderbytime = 45
                ElseIf settime = "FT" Then
                    orderbytime = 999
                Else
                    orderbytime = Convert.ToInt32(settime)
                End If


                '            Dim Scoresplit() As String = div.selectSingleNode(".//td[@class='score']").InnerText.Split("-") 'split the string for scores
                thescorestext = ""
                '            '' thescorestext += div.selectSingleNode(".//td[@class='time']").InnerText.Trim().Substring(0, 2) & "" 'set the time
                thescorestext += StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText) 'get home team name

                '            ' lblHTMLOutput.Text += mostused.gettheteams(StringOuttext(div.selectSingleNode(".//td[@class='home]").InnerText)) -redcard?

                '            'set the score with 0 being home team and 1 being away
                thescorestext += div.selectSingleNode(".//span[@class='ui-score-home']").InnerText & " - " & div.selectSingleNode(".//span[@class='ui-score-away']").InnerText

                '            '  lblHTMLOutput.Text += mostused.gettheteams(StringOuttext(div.selectSingleNode(".//td[@class='away']").InnerText))

                thescorestext += StringOutNumbers(div.selectSingleNode(".//span[@class='away-team-name']").InnerText) ' get away teams name

                '            'Check to see if anyrows in HomeTeam contain a value
                Dim count = scorestable.Rows.Cast(Of DataRow)().Where(Function(n) n("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText)).Count()
                'Find the count
                If count = 0 Then
                    'if not finished add row to the datatable as no results where found
                    '  scorestable.Rows.Add(StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText), "", thescorestext, "", "", "")


                    scorestable.Rows.Add(StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText), settime, thescorestext, "", "", orderbytime)

                Else  ' If count = 0 Then
                    'A matching result was found so now check to see if the Scores section of the result matches the value - thescorestext
                    For Each row As DataRow In scorestable.Rows
                        If row("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText) And row("Score").ToString() <> thescorestext Then
                            'add scores
                            row.SetField("Score", thescorestext)
                            row.SetField("Time", settime)
                            row.SetField("Lastgoaltime", DateTime.now())
                            row.SetField("Kickoff", orderbytime)
                        ElseIf row("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(".//span[@class='home-team-name']").InnerText) And row("Score").ToString() = thescorestext Then

                            ' add time
                            row.SetField("Time", settime)
                            row.SetField("Kickoff", orderbytime)

                        End If  ' else if  row("HomeTeam").ToString()  ...

                    Next ' For Each row As DataRow In scorestable.Rows

                End If ' else if  If count = 0 Then
            End If '    If Not div.selectSingleNode(".//span[@class='home-team-name']") Is Nothing 

        Next  ' For Each div As Object In 
	end if  ' If coll IsNot Nothing Then

        Return thescorestext
                                  

Open in new window


this will make your code more readable.
you might also use regions directives to be able to "collapse" code regions
http://msdn.microsoft.com/en-us/library/sd032a17.aspx
0
 
LVL 75

Expert Comment

by:käµfm³d 👽
ID: 39848652
Don't be afraid to add more function definitions when you have functions that start to get unwieldy. Having to scroll down 2 or three pages to see the rest of a block of code becomes tiring; so is scrolling around to try and locate a variable's declaration. Just make sure that functions you create make sense and logically encapsulate a piece of logic--don't create a new function solely for the sake of having another function have less lines.
0
 
LVL 10

Expert Comment

by:John Claes
ID: 39850753
Both previous comments are correct. I would suggest looking at them both ;-)

the foreach loop seems for me a legit function extraction possibility
the comments are a common issue, but try to write at least for every 2-3 lines 1 line of comment. It's hard (time consuming) for you now but easy for the future and the time-profit can be easly ten times

regards
0
 
LVL 20

Accepted Solution

by:
ElrondCT earned 2000 total points
ID: 39857582
One change I'd suggest for your scorestable loop:
                        For Each row As DataRow In scorestable.Rows
                            If row("HomeTeam").ToString() = StringOutNumbers(div.selectSingleNode(hometeam).InnerText) Then
                                If row("Score").ToString() <> thescorestext Then
                                    'add scores
                                    row.SetField("Score", thescorestext)
                                    row.SetField("Lastgoaltime", DateTime.Now())
                                End If
                                ' add time
                                row.SetField("Time", settime)
                                row.SetField("Kickoff", orderbytime)
                            End If
                        Next

Open in new window

When possible, avoid testing for the same thing twice, and setting the same thing twice. You may want to create a temporary variable holding the value of StringOutNumbers(div.selectSingleNode(hometeam).InnerText), since you use that several times.
0

Featured Post

On Demand Webinar: Networking for the Cloud Era

Ready to improve network connectivity? Watch this webinar to learn how SD-WANs and a one-click instant connect tool can boost provisions, deployment, and management of your cloud connection.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

For those of you who don't follow the news, or just happen to live under rocks, Microsoft Research released a beta SDK (http://www.microsoft.com/en-us/download/details.aspx?id=27876) for the Xbox 360 Kinect. If you don't know what a Kinect is (http:…
This article shows how to deploy dynamic backgrounds to computers depending on the aspect ratio of display
Michael from AdRem Software outlines event notifications and Automatic Corrective Actions in network monitoring. Automatic Corrective Actions are scripts, which can automatically run upon discovery of a certain undesirable condition in your network.…
In this video you will find out how to export Office 365 mailboxes using the built in eDiscovery tool. Bear in mind that although this method might be useful in some cases, using PST files as Office 365 backup is troublesome in a long run (more on t…
Suggested Courses

800 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question