none
Sub Versus Function

    General discussion

  • The code below adds strings to an existing list if there is no match for the first two TAB delimited fields.
    One Method uses a Sub and passes the existing list ByRef and adds to it if no match is found.

    The Other uses a Function that returns a Boolean True if match found, false if not, code runs the function and if it returns False, adds to the Existing.

    Is there any reason why one would be preferred to the other ?

    Code - Add to a blank Forms App:

    Option Strict On
    Public Class Form1
        Dim Existing As New List(Of String)
        Dim Additions As New List(Of String)
        WithEvents ButtonSub As New Button With {.Location = New Point(0, 187), .Text = "Use Sub", .Size = New Size(100, 25)}
        WithEvents ButtonFun As New Button With {.Location = New Point(120, 187), .Text = "Use Function", .Size = New Size(100, 25)}
        Dim RTB_Existing As New RichTextBox With {.Size = New Size(640, 90), .Location = New Point(0, 0)}
        Dim RTB_Addition As New RichTextBox With {.Size = New Size(640, 90), .Location = New Point(0, 95)}
        Dim RTB_Results As New RichTextBox With {.Size = New Size(640, 120), .Location = New Point(0, 220)}
        Private Sub TestDupe(ByVal StringToCheck As String, ByRef WhereToLook As List(Of String))
            Dim Found As Boolean = False
            Dim Fields() As String = StringToCheck.Split(vbTab.ToCharArray)
            Dim SearchFor As String = Fields(0) & vbTab & Fields(1)
            For Each S As String In WhereToLook
                If S.StartsWith(SearchFor) Then
                    Found = True
                End If
            Next
            If Not Found Then
                WhereToLook.Add(StringToCheck)
            End If
        End Sub
        Private Function IsDuplicate(StringToCheck As String, WhereToLook As List(Of String)) As Boolean
            Dim Fields() As String = StringToCheck.Split(vbTab.ToCharArray)
            Dim SearchFor As String = Fields(0) & vbTab & Fields(1)
            For Each S As String In WhereToLook
                If S.StartsWith(SearchFor) Then
                    Return True
                End If
            Next
            Return False
        End Function
        Private Sub ShowResults()
            RTB_Results.Clear()
            For Each S As String In Existing
                RTB_Results.AppendText(S & vbNewLine)
            Next
        End Sub
        Private Sub AddData()
            RTB_Existing.Clear()
            RTB_Addition.Clear()
            RTB_Results.Clear()
            RTB_Existing.Refresh()
            RTB_Addition.Refresh()
            RTB_Results.Refresh()
    
            Existing.Clear()
            Additions.Clear()
            System.Threading.Thread.Sleep(100)
            Existing.Add("Criminal Minds" & vbTab & "S12E01" & vbTab & "9/28/2016" & vbTab & "The Crimson King" & vbTab & "True")
            Existing.Add("Criminal Minds" & vbTab & "S12E03" & vbTab & "10/12/2016" & vbTab & "Taboo" & vbTab & "True")
            Existing.Add("Criminal Minds" & vbTab & "S12E04" & vbTab & "10/26/2016" & vbTab & "Keeper" & vbTab & "True")
            Existing.Add("Criminal Minds" & vbTab & "S12E06" & vbTab & "11/16/2016" & vbTab & "Elliott's Pond" & vbTab & "True")
            Existing.Add("Criminal Minds" & vbTab & "S12E07" & vbTab & "11/30/2016" & vbTab & "Mirror Image" & vbTab & "True")
            Additions.Add("Criminal Minds" & vbTab & "S12E02" & vbTab & "10/5/2016" & vbTab & "Sick Day" & vbTab & "True")
            Additions.Add("Criminal Minds" & vbTab & "S12E03" & vbTab & "10/12/2016" & vbTab & "Taboo" & vbTab & "True")
            Additions.Add("Criminal Minds" & vbTab & "S12E05" & vbTab & "11/9/2016" & vbTab & "The Anti-Terror Squad" & vbTab & "True")
            Additions.Add("Criminal Minds" & vbTab & "S12E07" & vbTab & "11/30/2016" & vbTab & "Mirror Image" & vbTab & "True")
            For Each S As String In Existing
                RTB_Existing.AppendText(S & vbNewLine)
            Next
            For Each S As String In Additions
                RTB_Addition.AppendText(S & vbNewLine)
            Next
        End Sub
        Private Sub Form1_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load
            Me.Size = New Size(660, 440)
            Me.Font = New Font("Consolas", 8)
            Me.Controls.Add(ButtonSub)
            Me.Controls.Add(ButtonFun)
            Me.Controls.Add(RTB_Existing)
            Me.Controls.Add(RTB_Addition)
            Me.Controls.Add(RTB_Results)
            AddData()
        End Sub
    
        Private Sub Sub_Click(sender As System.Object, e As System.EventArgs) Handles ButtonSub.Click
            AddData()
            For Each S As String In Additions
                TestDupe(S, Existing)
            Next
            Existing.Sort()
            ShowResults()
        End Sub
    
        Private Sub Func_Click(sender As System.Object, e As System.EventArgs) Handles ButtonFun.Click
            AddData()
            For Each S As String In Additions
                If Not IsDuplicate(S, Existing) Then
                    Existing.Add(S)
                End If
            Next
            Existing.Sort()
            ShowResults()
        End Sub
    End Class
    
    

    Saturday, March 11, 2017 2:25 AM

All replies

  • I think it's all about the nature of the object. When you use a function why not just call it GetWhatever because most of the time that's why. But when you really think about it, it's simple to create an appropriateness rating.

    First off, when you need a second object returned from a procedure it does not matter if it's a sub or function, you're better off using a parameter. I cannot argue if a function returning an array of different types is better than using parameters because I've never really compared them, but I think it's safe to say by the end of the procedure I will prefer English variables rather than an array index that needs sorting.

    So the appropriateness is simple to work out logically. First, if you only need to return one thing, use a function. Plus, it's handy to make a variable equal to a function. Second, if you need two items maybe a sub. Third, the creation of the return object is paramount so, use a function and return an array of objects.

    This is just common sense to me. If you see a flaw, I would appreciate the correction.



    Saturday, March 11, 2017 3:37 AM
  • George - To be perfectly honest, no disrespect meant, I have no idea what you mean. The example I posted is a real project I am involved in.

    The function part returns True if a match is found in what I call the critical fields (Name of Show, Season/Episode). If it returns False, the show data is added. The return from this Function is either True or False and there is no need to assign that to a variable since it does not get re-used. Returning an Array or List of Strings, as far as I know, is wasteful because a new Array/List would get created every time it is called. I prefer lists myself. I can't use the Distinct Function because the Strings can be slightly different but for these purposes they must be classified as the same. Example:

    Criminal Minds {TAB} S12E01 {TAB} 9/28/2016 {TAB} The Crimson King {TAB} True
    Criminal Minds {TAB} S12E01 {TAB} 09/28/2016 {TAB} The Crimson King {TAB} True
    Criminal Minds {TAB} S12E01 {TAB} 9/28/2016 {TAB} The Crimson King {TAB} False
    Criminal Minds {TAB} S12E01 {TAB} 2016/9/28 {TAB} The Crimson King {TAB} True
    Criminal Minds {TAB} S12E01 {TAB} 2016/09/28 {TAB} The Crimson King {TAB} True
    All 5 of those above must be calculated as identical for this purpose.
    Criminal Minds {TAB} S12E01        {TAB} 9/28/2016 {TAB} The Crimson King {TAB} True
    Name of Show   {TAB} Season/Episode{TAB} Air Date  {TAB} Show Title       {TAB} Viewed?
    Only the Name and Season/Episode are compared. A future revision may check the Air Date and Title and offer the user a choice of which to keep.


    The Sub does the testing and adding all in one code block. I guess my "question" is more about using ByRef than anything else.

    The Data has to be sorted because it can arrive out of order and the user needs to see it in order. I don't see any possible way around that.

    Saturday, March 11, 2017 4:19 AM
  • Sorry, for not getting deeper. I can and will continue.

    The pattern you use makes the flow of your code completely different. I'm sure there is some paradigm for this kind of thing. In the end I've used both significantly.

    What can I say about it...

    When you separate the code, it's nice to have, so for that reason, maybe even object orientation, you may want to do it that way. But when you wire something up for connectivity (whatever you may call it), it's the pattern in ensuring new registry variables can be added to an application in an update, for example. You may not need that extra little bit so, leave it in there. But when you need just that bit again, you need to remember that that code is there and needs separation.




    Saturday, March 11, 2017 7:42 AM
  • Devon, 

    The reason they created functional languages is because of some persons want to avoid to use subs (void in C type languages) as method. 

    However, the same exist in fact with functions and properties. If you look at the DateTime struct it is very discus honorable if some properties are in fact no functions. 

    There was a time when was discussed that C# also would become functional, however it was a personal preference from Hejslberg not to do that. In the Microsoft Language range is therefore F# created. 

    Constructions as the TryCast where a field is passed inside the sub to get a value are in many persons eyes (including mine) from the days the cars where still pulled by horses. 

    But in fact it is just all about personal preferences. 


    Success
    Cor

    Saturday, March 11, 2017 8:32 AM
  • If it matters a simple function seems to be 5 to 10 percent faster than the byref sub.

            

    Public Class Form3
        Private sw As New Stopwatch
    
        Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
            'make a string 
            Dim str As String = Nothing
            For i = 1 To 100
                str &= "abcdefghijklmnopqrstuvwxyz"
            Next
    
            sw.Reset()
            sw.Start()
            Dim lettercount As Integer
            For i = 1 To 10000
                CountLetters1("a", str, lettercount)
            Next
    
            sw.Stop()
            Dim t1 As Single = sw.ElapsedMilliseconds
    
    
            sw.Reset()
            sw.Start()
    
            For i = 1 To 10000
                lettercount = CountLetters2("a", str)
            Next
    
            sw.Stop()
            Dim t2 As Single = sw.ElapsedMilliseconds
    
            Label1.Text = "Sub: " & t1.ToString & "   Fun: " & t2.ToString & vbLf & vbLf &
                (100 - (100 * t2 / t1)).ToString("f0") & "%"
    
    
        End Sub
    
        Private Sub CountLetters1(ByVal theLetter As String, ByVal theString As String, ByRef letterCount As Integer)
            letterCount = 0
    
            For i = 0 To theString.Length - 1
    
                If theString.Substring(i, 1) = theLetter Then
                    letterCount += 1
                End If
            Next
    
        End Sub
    
        Private Function CountLetters2(ByVal theLetter As String, ByVal theString As String) As Integer
            Dim letterCount As Integer = 0
    
            For i = 0 To theString.Length - 1
    
                If theString.Substring(i, 1) = theLetter Then
                    letterCount += 1
                End If
            Next
    
            Return letterCount
    
        End Function
    End Class

    Saturday, March 11, 2017 12:45 PM
  • tommy - what did you get for results?  As written, your test on my machine, was taking a very long time.  When I looked at what it was doing I understood why, there were lots and lots of temporary strings being created.  Since this wasn't a string performance measurement activity I modified the test.  The methods performed similarly.

        Private sw As New Stopwatch
        Private i As Integer
        Private SUBlettercount As Integer
        Private FUNlettercount As Integer
    
        Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
            Label1.Text = "GO"
            Label1.Refresh()
    
            'make a string 
            Dim str As String = Nothing
            For i = 1 To 100
                str &= "abcdefghijklmnopqrstuvwxyz"
            Next
    
            Dim tries As Integer = 5000
            Dim t1 As Single
            Dim t2 As Single
    
            sw.Reset()
            sw.Start()
    
            For i = 1 To tries
                CountLetters1("a", str, SUBlettercount)
            Next
    
            sw.Stop()
            t1 = sw.ElapsedMilliseconds
    
            sw.Reset()
            sw.Start()
    
            For i = 1 To tries
                FUNlettercount = CountLetters2("a", str)
            Next
    
            sw.Stop()
            t2 = sw.ElapsedMilliseconds
    
            Label1.Text = "Sub: " & t1.ToString("n0") & "   Fun: " & t2.ToString("n0")
    
        End Sub
    
        Private Sub CountLetters1(theLetter As String, theString As String, ByRef letterCount As Integer)
            letterCount = (From c In theString Where c = theLetter Select c).Count
        End Sub
    
        Private Function CountLetters2(theLetter As String, theString As String) As Integer
            Dim letterCount As Integer = (From c In theString Where c = theLetter Select c).Count
    
            Return letterCount
        End Function
    


    "Those who use Application.DoEvents() have no idea what it does and those who know what it does never use it." - MSDN User JohnWein    Multics - An OS ahead of its time.

    Saturday, March 11, 2017 2:50 PM
  • Devon - looking at the two methods I noticed that TestDup looks like it should have an Exit For when there is a match.

    What you have is basic functionality I think.  You have 'is duplicate', 'add', and a combination of the two 'add if not dupe' as a Function or Sub.  The add if not dupe as Sub seems out of place, forced if you will. 

        Private Existing As New List(Of String)
        Private test() As String = New String() {"Lorem", "ipsum", "dolor", "sit", "mollit", "anim", "id", "est", "laborum"}
    
        Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
            Dim adds() As String = New String() {"foo", "bar", "dolor", "sit", "test"}
    
            Dim existed As Boolean = False
    
            TestData()
            For Each s As String In adds
                existed = AddIfNotDupe(s, Existing)
                If existed Then
                    'Stop
                End If
            Next
            Debug.WriteLine(Existing.Count)
    
            TestData()
            For Each s As String In adds
                AddIfNotDupe(s, Existing, existed)
                If existed Then
                    'Stop
                End If
            Next
            Debug.WriteLine(Existing.Count)
        End Sub
    
        Private Function AddIfNotDupe(newItemQ As String, exst As List(Of String)) As Boolean
            Dim existed As Boolean = isDupe(newItemQ, exst)
            If Not existed Then
                AddIt(newItemQ, exst)
            End If
            Return existed
        End Function
    
        Private Sub AddIfNotDupe(newItemQ As String, exst As List(Of String), ByRef existed As Boolean)
            existed = isDupe(newItemQ, exst)
            If Not existed Then
                AddIt(newItemQ, exst)
            End If
        End Sub
    
        Private Function isDupe(newItemQ As String, exstng As List(Of String)) As Boolean
            Dim exists As Boolean = False
            For Each itm As String In exstng
                If itm = newItemQ Then
                    exists = True
                    Exit For
                End If
            Next
            Return exists
        End Function
    
        Private Sub AddIt(newItem As String, exst As List(Of String))
            exst.Add(newItem)
        End Sub
    
        Private Sub TestData()
            Existing.Clear()
            Existing.AddRange(test)
        End Sub


    "Those who use Application.DoEvents() have no idea what it does and those who know what it does never use it." - MSDN User JohnWein    Multics - An OS ahead of its time.


    • Edited by dbasnett Saturday, March 11, 2017 3:55 PM
    Saturday, March 11, 2017 3:46 PM
  • Devon,

    You would use the function version if you would ever need to test for the duplicate without modifying the list, or if the calling routine needed to know if the list changed.  If you never need to do any of that, then the sub version is preferable because it captures the entire operation within a single method.  But truly either method could be written to encapsulate the entire operation because the List(Of String) is a reference type so it is always passed ByRef, with or without the keyword.  Either method could modify the list passed in the parameter.  The real question is whether or not you need the Boolean result and from the look of the way the code is being used, you do not need it.

    I can't think of any technical reason why you would choose on route over the other; it all comes down to intended usage and personal preference.


    Reed Kimble - "When you do things right, people won't be sure you've done anything at all"

    Saturday, March 11, 2017 4:16 PM
    Moderator
  • tommy - what did you get for results?  As written, your test on my machine, was taking a very long time.  When I looked at what it was doing I understood why, there were lots and lots of temporary strings being created.  Since this wasn't a string performance measurement activity I modified the test.  The methods performed similarly.

        Private sw As New Stopwatch
        Private i As Integer
        Private SUBlettercount As Integer
        Private FUNlettercount As Integer
    
        Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
            Label1.Text = "GO"
            Label1.Refresh()
    
            'make a string 
            Dim str As String = Nothing
            For i = 1 To 100
                str &= "abcdefghijklmnopqrstuvwxyz"
            Next
    
            Dim tries As Integer = 5000
            Dim t1 As Single
            Dim t2 As Single
    
            sw.Reset()
            sw.Start()
    
            For i = 1 To tries
                CountLetters1("a", str, SUBlettercount)
            Next
    
            sw.Stop()
            t1 = sw.ElapsedMilliseconds
    
            sw.Reset()
            sw.Start()
    
            For i = 1 To tries
                FUNlettercount = CountLetters2("a", str)
            Next
    
            sw.Stop()
            t2 = sw.ElapsedMilliseconds
    
            Label1.Text = "Sub: " & t1.ToString("n0") & "   Fun: " & t2.ToString("n0")
    
        End Sub
    
        Private Sub CountLetters1(theLetter As String, theString As String, ByRef letterCount As Integer)
            letterCount = (From c In theString Where c = theLetter Select c).Count
        End Sub
    
        Private Function CountLetters2(theLetter As String, theString As String) As Integer
            Dim letterCount As Integer = (From c In theString Where c = theLetter Select c).Count
    
            Return letterCount
        End Function


    "Those who use Application.DoEvents() have no idea what it does and those who know what it does never use it." - MSDN User JohnWein    Multics - An OS ahead of its time.

    dBase,

    Well the times are shown in the image as millisecs. The answer is letterCount =100 for both tests. Is that what you mean?

    Yes I used strings to take time. Since both routines do the same thing I would think they are the same except for the diff between function and sub by ref? I don't see any difference between the two routines other than the diff between function and sub by ref.

    I am not sure what you mean or I am missing something I guess?

    Here are my average results of your example which seem similar to my example:

    Note I changed your example to 10000 loops from the 5000 you show, same as my example and your example is slower than my example? But I am not sure what your code does I don't understand that loop shorthand you used.

    You have to run the test several times. The first time is normally slower. I guess its caching.

    :)

    Saturday, March 11, 2017 5:43 PM
  • "I can't think of any technical reason why you would choose on route over the other; it all comes down to intended usage and personal preference."

    That is really what I was looking for. The list itself will never be more than a few hundred strings and gets updated once a week, so absolute performance is not much of an issue. I tested this with almost 1300 entries and the Sub takes 290-300 ms while the function takes 150-160 ms. Adding an Exit For to the sub makes its speed virtually the same as the function.

    At some point as I mentioned, I may need to make this compare more flexible and I think the function will be necessary for that, i.e. can return a Value describing what fields match
    Show Name match = 1
    Season/Episode match = 2
    Air Date match = 4
    Title match = 8
    Flag match = 16


    • Edited by Devon_Nullman Saturday, March 11, 2017 5:54 PM Added Timing Information
    Saturday, March 11, 2017 5:52 PM
  • Class MainWindow
    
        Private Sub MainWindow_Loaded(sender As Object, e As RoutedEventArgs) Handles Me.Loaded
    
            MsgBox(GetValue5(0))
    
        End Sub
    
        Private Function GetValue5()
    
            Return {"Hello", 5, "Uh"}
    
        End Function
    
    End Class

    Saturday, March 11, 2017 6:43 PM
  • PS For fun I added the no routine in line code example. It seems the function adds no time beyond just using the code without a function?

    Wait...

    Ok well I reversed the order of the test and got reverse results as shown below. So that may be due to what you mean and see dbasnett.

    So, that's the time test I did for what its worth.

    Not a big difference either way apparently.

    Option Strict On
    Public Class Form5
        'sub routine calls time test 3
        Private sw As New Stopwatch
    
    
        Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
            'make a string 
            Dim theString As String = Nothing
            For i = 1 To 100
                theString &= "abcdefghijklmnopqrstuvwxy3z"
            Next
    
            Dim lettercount As Integer = 0
            Dim theLetter As String = "a"
    
    
    
    
            '*****  function test ***** 
            sw.Reset()
            sw.Start()
    
            For i = 1 To 10000
                lettercount = CountLettersFun(theLetter, theString)
            Next
    
            sw.Stop()
            Dim tFun As Single = sw.ElapsedMilliseconds
    
    
            '***** byref sub test ***** 
            sw.Reset()
            sw.Start()
    
            For i = 1 To 10000
                CountLettersSub(theLetter, theString, lettercount)
            Next
    
            sw.Stop()
            Dim tSub As Single = sw.ElapsedMilliseconds
    
            '*****  in line code test ***** 
            sw.Reset()
            sw.Start()
    
            For i = 1 To 10000
                lettercount = 0
    
                For j = 0 To theString.Length - 1
                    If theString.Substring(j, 1) = theLetter Then
                        lettercount += 1
                    End If
                Next
            Next
    
            sw.Stop()
            Dim tInLine As Single = sw.ElapsedMilliseconds
    
    
    
            Label1.Text =
                "Byref Sub: " & tSub.ToString & vbLf & vbLf &
                "Function:  " & tFun.ToString & vbLf & vbLf &
                "In Line: " & tInLine.ToString
    
    
        End Sub
    
        Private Sub CountLettersSub(ByVal theLetter As String, ByVal theString As String, ByRef letterCount As Integer)
            letterCount = 0
    
            For i = 0 To theString.Length - 1
                If theString.Substring(i, 1) = theLetter Then
                    letterCount += 1
                End If
            Next
    
        End Sub
    
        Private Function CountLettersFun(ByVal theLetter As String, ByVal theString As String) As Integer
            CountLettersFun = 0
    
            For i = 0 To theString.Length - 1
                If theString.Substring(i, 1) = theLetter Then
                    CountLettersFun += 1
                End If
            Next
    
        End Function
    End Class

    PS Yeah, I put the inline test first and now it is the slowest and the sub and function are the same. So not sure whats happening there.


    Saturday, March 11, 2017 7:00 PM
  • You are right, I just decided to remove my reply


    Success
    Cor

    Saturday, March 11, 2017 11:06 PM
  • This is sort of like the decision of whether to use method overloading or use one or more optional parameters:

    Sometimes it's obvious which way to go and other times it'll work either way.


    "One who has no vices also has no virtues..."

    Saturday, March 11, 2017 11:18 PM
  • You are right, I just decided to remove my reply


    Success
    Cor

    Me too

    Sunday, March 12, 2017 1:48 AM