CODEWITHSUNDEEP
.netvb.netoop
aape
aape

Reputation: 517

in .net, if a function returns an object, is it wrong to just use the function, _as_ an object?

I've got a f(x) that returns a collection of user controls. .Net lets me just treat the f(x) name as the collection. Is there something wrong with doing so?

ex)

Private Function GetCcB() As Collection(Of Reports_ucColumn)
    Dim cc As New Collection(Of Reports_ucColumn)
    cc.Add(Me.ucColumn0)
    cc.Add(Me.ucColumn1)
    cc.Add(Me.ucColumn2)
    Return cc
End Function

Then later on, all use the properties on the objects in the collection, like this:

    For i As Integer = 0 To csB.Count - 1
        If csB(i).Contains("Invalid") Or csB(i).Contains("Duplicate") Then
            GetCcB.Item(i).isValid = False
            GetCcB.Item(i).title = csB(i)
            If csB(i).Contains("Invalid") Then : GetCcB.Item(i).errCode = "005W"
            Else : GetCcB.Item(i).errCode = "005W" 'todo - chg this once we get a duplicate col err code
            End If
        Else
            GetCcB.Item(i).isValid = True
            GetCcB.Item(i).title = csB(i)
            GetCcB.Item(i).errCode = String.Empty
        End If

Upvotes: 1

Views: 344

Answers (3)

Guffa
Guffa

Reputation: 700322

By calling the method each time that you use the collection, what your code is effectively doing is:

For i As Integer = 0 To csB.Count - 1
   Dim cc As Collection(Of Reports_ucColumn)
   If csB(i).Contains("Invalid") Or csB(i).Contains("Duplicate") Then
      cc = New Collection(Of Reports_ucColumn)
      cc.Add(Me.ucColumn0)
      cc.Add(Me.ucColumn1)
      cc.Add(Me.ucColumn2)
      cc.Item(i).isValid = False
      cc = New Collection(Of Reports_ucColumn)
      cc.Add(Me.ucColumn0)
      cc.Add(Me.ucColumn1)
      cc.Add(Me.ucColumn2)
      cc.Item(i).title = csB(i)
      If csB(i).Contains("Invalid") Then
         cc = New Collection(Of Reports_ucColumn)
         cc.Add(Me.ucColumn0)
         cc.Add(Me.ucColumn1)
         cc.Add(Me.ucColumn2)
         cc.Item(i).errCode = "005W"
      Else
         cc = New Collection(Of Reports_ucColumn)
         cc.Add(Me.ucColumn0)
         cc.Add(Me.ucColumn1)
         cc.Add(Me.ucColumn2)
         cc.Item(i).errCode = "005W" 'todo - chg this once we get a duplicate col err code
      End If
   Else
      cc = New Collection(Of Reports_ucColumn)
      cc.Add(Me.ucColumn0)
      cc.Add(Me.ucColumn1)
      cc.Add(Me.ucColumn2)
      cc.Item(i).isValid = True
      cc = New Collection(Of Reports_ucColumn)
      cc.Add(Me.ucColumn0)
      cc.Add(Me.ucColumn1)
      cc.Add(Me.ucColumn2)
      cc.Item(i).title = csB(i)
      cc = New Collection(Of Reports_ucColumn)
      cc.Add(Me.ucColumn0)
      cc.Add(Me.ucColumn1)
      cc.Add(Me.ucColumn2)
      cc.Item(i).errCode = String.Empty
   End If
Next

You would never write code like that, would you?

You can turn the function into a property with lazy initialisation. Then it's ok to use it over and over, as the collection is only created once:

Private _cc As Collection(Of Reports_ucColumn)

Private Property CcB As Collection(Of Reports_ucColumn)
   Get
      If _cc Is Nothing Then
         _cc = New Collection(Of Reports_ucColumn)
         _cc.Add(Me.ucColumn0)
         _cc.Add(Me.ucColumn1)
         _cc.Add(Me.ucColumn2)
      End If
      Return _cc
   End Get
End Property

(Ideally the property should be public in a class and the _cc variable should be private, encapsulated in the class so that only the property could access it.)

Semantically this works fine also. A function generally does a bit of work, so you shouldn't call it over and over if you want to use the result over and over. A property on the other hand usually doesn't do much work, and if the result needs some processing to be created, the property usually does something like lazy initialisation or pre-initialisation to make sure that the work isn't done more than neccessary.

Upvotes: 1

squillman
squillman

Reputation: 13641

You'd want to use a singleton instead. It's a static class with a property that returns a private field of the class.

Upvotes: 1

J. Steen
J. Steen

Reputation: 15578

Well, you keep creating new collections in memory each time you call GetCcB, so I would recommend using a variable instead. =)

Edit: In essence, the object (collection) that GetCcB returns is different (though not in content) each time you call it and takes up additional memory on the heap which has to be collected by the garbage collector when it makes a pass. Better to just grab the collection as a variable and use that.

Upvotes: 1

Related Questions