Solved

VB.NET -> Cutting down on If statements

Posted on 2014-02-10
4
311 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 500 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

Containers and Docker for Everyone

Containers are an incredibly powerful technology that can provide you and/or your engineering team with huge productivity gains. Using containers, you can deploy, back up, replicate, and move apps and their dependencies quickly and easily.

Question has a verified solution.

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

Real-time is more about the business, not the technology. In day-to-day life, to make real-time decisions like buying or investing, business needs the latest information(e.g. Gold Rate/Stock Rate). Unlike traditional days, you need not wait for a fe…
The article shows the basic steps of integrating an HTML theme template into an ASP.NET MVC project
In this video, viewers will be given step by step instructions on adjusting mouse, pointer and cursor visibility in Microsoft Windows 10. The video seeks to educate those who are struggling with the new Windows 10 Graphical User Interface. Change Cu…
Michael from AdRem Software explains how to view the most utilized and worst performing nodes in your network, by accessing the Top Charts view in NetCrunch network monitor (https://www.adremsoft.com/). Top Charts is a view in which you can set seve…

688 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