Solved

VB.NET -> Cutting down on If statements

Posted on 2014-02-10
4
300 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
4 Comments
 
LVL 142

Expert Comment

by:Guy Hengel [angelIII / a3]
Comment Utility
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 74

Expert Comment

by:käµfm³d 👽
Comment Utility
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
Comment Utility
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 500 total points
Comment Utility
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

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

IntroductionWhile developing web applications, a single page might contain many regions and each region might contain many number of controls with the capability to perform  postback. Many times you might need to perform some action on an ASP.NET po…
Creating an analog clock UserControl seems fairly straight forward.  It is, after all, essentially just a circle with several lines in it!  Two common approaches for rendering an analog clock typically involve either manually calculating points with…
Sending a Secure fax is easy with eFax Corporate (http://www.enterprise.efax.com). First, Just open a new email message.  In the To field, type your recipient's fax number @efaxsend.com. You can even send a secure international fax — just include t…
Illustrator's Shape Builder tool will let you combine shapes visually and interactively. This video shows the Mac version, but the tool works the same way in Windows. To follow along with this video, you can draw your own shapes or download the file…

771 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

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now