[lar3ry]

I am rewriting a Satellte Solar Transit prediction program, and am trying to make it more object oriented than the VB6 I originally wrote it in.

I am trying to return a List(Of T) from a function, as follows:

Public Class Observers

    Private Structure Observer
        Dim Name As String
        Dim ObsLat As Double
        Dim ObsLong As Double
        Dim ObsElev As Integer
        Dim UTCOffset As Int16
        Dim ObsDst As Boolean
        Dim ObsEmail As String
    End Structure

    Public Function loadObservers() As List(Of Observer)  'Error here. Observer is highlighted
        Dim sr As New StreamReader("Observers.txt")
        Dim obslist As New List(Of Observer)
        Dim obsdata As New Observer
        If File.Exists("Observers.txt") Then
            Dim s() As String
            Do
                s = Split(sr.ReadLine(), ",")
                obsdata.Name = s(0).Trim
                obsdata.ObsLat = CDbl(s(1))
                obsdata.ObsLong = CDbl(s(2))
                obsdata.ObsElev = CInt(s(3))
                obsdata.UTCOffset = CShort(s(4))
                obsdata.ObsDst = CBool(s(5))
                obsdata.ObsEmail = s(6).Trim
                obslist.Add(obsdata)
            Loop Until sr.Peek = -1
        End If
        loadObservers = obslist
    End Function
End Class

I get an error in the Function Declaration. List(Of Observer). Observer is highlighted, and the errors read:

error BC36666: 'SatTransNew.Observers.Public Function loadObservers() As System.Collections.Generic.List(Of Observers.Observer)' is not accessible in this context because the return type is not accessible.

error BC30508: 'loadObservers' cannot expose type 'Observer' in namespace 'SatTransNew' through class 'Observers'.

I tried moving the Dim obslist As New List(Of Observer) outside the function, but it didn't help.

Is it possible to Return a List(Of T), or should I go with an array?

Edit: I just tried making the Structure declaration Publlic, and it now compiles. Is that the answer? I was going to keep that private, though thinking about it, I can't see why it matters.\

This post has been edited by lar3ry: 20 November 2012 - 01:44 PM

[lucky3]

Change line 31 to Return obslist.
Edit: and move line 16 between 19 and 20 (below Do).

This post has been edited by lucky3: 20 November 2012 - 01:56 PM

[lar3ry]

lucky3, on 20 November 2012 - 02:53 PM, said:

Change line 31 to Return obslist.
Edit: and move line 16 between 19 and 20 (below Do).

Hmm... line 31..is same thing, right? Assign value to return to name of function? For whatever reason, I've always preferred that method.

Line 16... that would create a new instance of obsdata for each iteration. Do I need to Dispose them?

Thanks!

[_HAWK_]

Don't you want the Dim obsdata As New Observer inside of the loop - more than one observer?

Public Class Observers

    Private Structure Observer
        Dim Name As String
        Dim ObsLat As Double
        Dim ObsLong As Double
        Dim ObsElev As Integer
        Dim UTCOffset As Int16
        Dim ObsDst As Boolean
        Dim ObsEmail As String
    End Structure

    Public Function loadObservers() As List(Of Observer)  'Error here. Observer is highlighted
        Dim sr As New StreamReader("Observers.txt")
        Dim obslist As New List(Of Observer)
        If File.Exists("Observers.txt") Then
            Dim s() As String
            Do
                s = Split(sr.ReadLine(), ",")
                Dim obsdata As New Observer ' <- here
                obsdata.Name = s(0).Trim
                obsdata.ObsLat = CDbl(s(1))
                obsdata.ObsLong = CDbl(s(2))
                obsdata.ObsElev = CInt(s(3))
                obsdata.UTCOffset = CShort(s(4))
                obsdata.ObsDst = CBool(s(5))
                obsdata.ObsEmail = s(6).Trim
                obslist.Add(obsdata)
            Loop Until sr.Peek = -1
        End If
        loadObservers = obslist
    End Function
End Class

[lar3ry]

_HAWK_, on 20 November 2012 - 05:50 PM, said:

Don't you want the Dim obsdata As New Observer inside of the loop - more than one observer?

That's what lucky3 mentioned, so I did move it inside the loop. The result is the same, when I call it from a Form, I get a list of Observers.

Yes, there's more than one observer, and the user will have the ability to add or delete observers.

I guess what I'm wondering is: does it matter if I re-use a single Observer struct? I use it, add it to the list, and then re-use it, adding it to the list again (and again...). From the result (same return whether inside or outside the loop), I would assume that the List contains a copy of the Observer data, in the appropriate format, at the time it was added.

I did move the line Dim obslist As New List(Of Observer) outside the function (and changed it to Private _obslist As New List(Of Observer), in order to keep a copy of it, ready to accept more observers and to save to disk.

I've been programming for MANY years, but I'm a relative newcomer to OOP.

[_HAWK_]

It is fine to reuse the variable as long as you initialize it at the beginning of each iteration. The List is holding the reference for the object you created.

[lucky3]

lar3ry, on 20 November 2012 - 10:55 PM, said:

Hmm... line 31..is same thing, right? Assign value to return to name of function? For whatever reason, I've always preferred that method.

There is a subtle difference if you use function's name as a return value, or if you use the Return statement. Take a look at the example:

Module Module1

    Sub Main()
        Dim a As Integer = UsingFunctionsName(11)
        Console.WriteLine("Using function's name as return value: " & a.ToString)

        Dim b As Integer = UsingReturnStatement(11)
        Console.WriteLine("Using Return statement as return value: " & b.ToString)

        Console.ReadLine()
    End Sub

    Function UsingFunctionsName(input As Integer) As Integer
        Dim a As Integer = input
        Dim b As Integer

        If a > 10 Then
            b = 2
            a = a * b * b
            UsingFunctionsName = a
        End If

        If a > 40 Then
            b = 3
            a = a * b - 130
            UsingFunctionsName = a
        End If

        If a <= 10 Then
            b = 5
            UsingFunctionsName = a + b
        End If
    End Function

    Function UsingReturnStatement(input As Integer) As Integer
        Dim a As Integer = input
        Dim b As Integer

        If a > 10 Then
            b = 2
            a = a * b * b
            Return a
        End If

        If a > 40 Then
            b = 3
            a = a * b - 130
            Return a
        End If

        If a <= 10 Then
            b = 5
            Return a + b
        End If
    End Function
End Module

I'd say that using Return statement prevents possible confusion, and unexpected return values from later operations inside the function. In your particular case, there was no difference though.

Quote

Line 16... that would create a new instance of obsdata for each iteration. Do I need to Dispose them?

No need to dispose them, because they are encapsulated in the list, which is managed by .NET garbage collector (as every .NET member is).

The difference between instantiating object inside the loop, and adding each to the list, compared to instantiating object outside the loop, and adding reference of the same object to the list, is easily seen by this simple example:

Module Module1

    Sub Main()
        Dim listOfIntegerHolderObjects As List(Of IntegerHolder)
        listOfIntegerHolderObjects = FillTheListUseOutsideInstantiationOfObject()

        Console.WriteLine("From function, where object is instantiated OUTSIDE the for loop, and added to list:")
        For Each HolderObject As IntegerHolder In listOfIntegerHolderObjects
            Console.Write(HolderObject.Value.ToString() & ", ")
        Next
        Console.WriteLine()
        Console.WriteLine()

        listOfIntegerHolderObjects = FillTheListUseInsideInstantiationOfObject()
        Console.WriteLine("From function, where object is instantiated INSIDE the for loop, and added to list:")
        For Each HolderObject As IntegerHolder In listOfIntegerHolderObjects
            Console.Write(HolderObject.Value.ToString() & ", ")
        Next
        Console.WriteLine()

        Console.ReadLine()
    End Sub

    Function FillTheListUseOutsideInstantiationOfObject() As List(Of IntegerHolder)
        Dim holder As New IntegerHolder
        Dim returnMe As New List(Of IntegerHolder)

        For n = 1 To 100
            holder.Value = n
            returnMe.Add(holder)
        Next

        Return returnMe
    End Function

    Function FillTheListUseInsideInstantiationOfObject() As List(Of IntegerHolder)
        Dim returnMe As New List(Of IntegerHolder)

        For n = 1 To 100
            Dim holder As New IntegerHolder With {.Value = n}
            returnMe.Add(holder)
        Next

        Return returnMe
    End Function
End Module

Class IntegerHolder
    Public Value As Integer
End Class

[CodingSup3rnatur@l-360]

The original error was indeed because the Observer structure was private, but the loadObservers() method was public. This means that the Observer structure is not accessible outside of the Observers class, and the loadObservers() method is accessible outside of the Observers class.

Why is this a problem? It is a problem because you were returning a list of Observer instances, meaning that other classes outside of the Observers class can now gain access to the Observer instances by calling the loadObservers() method, thereby violating the private access modifier on the Observer structure.

Making the Observer structure public will therefore solve the problem, as you found.


Regarding the creating of the Observer instance inside or outside the loop, it makes no difference to the end result (i.e. what is in the list after the loop has finished executing) whether Observer is created inside or outside the loop, simply because the Observer type is defined as a structure, meaning that it is a value type. Therefore, every time you add the structure to the list, its entire contents are copied, and put into the list.

Generally speaking, the vast majority of the types you make should be classes, rather than structures. Structures have their uses here and there, but unless you fully understand them, along with the reason you want to use a structure, you probably shouldn't even consider using them.

You should change the Observer type to be a class rather than a structure. If you do that though, you will need to move the creating of the Observer instance to inside the loop, as the others have said.

Also, if you do define a structure like you have here though, at least do yourself a favor, and make it immutable

This post has been edited by CodingSup3rnatur@l-360: 21 November 2012 - 06:21 AM

[lucky3]

I must have missed that Observer is structure not class. My bad!

[lar3ry]

lucky3, on 21 November 2012 - 01:29 AM, said:

I'd say that using Return statement prevents possible confusion, and unexpected return values from later operations inside the function. In your particular case, there was no difference though.

I do sometimes use a Return(), but prefer the other way, for the same reasons I dislike GoTo. I'd rather structure my code to allow for a default return value and a single exit by falling through the End Function. If the code is such that structuring it that way is difficult, I'll sometimes use Return() to exit early. The other exception is if I want to return an array, but then, I'll try to use a variable (retval, for example) and an explicit Return.

[lar3ry]

CodingSup3rnatur@l-360, on 21 November 2012 - 04:41 AM, said:

Generally speaking, the vast majority of the types you make should be classes, rather than structures. Structures have their uses here and there, but unless you fully understand them, along with the reason you want to use a structure, you probably shouldn't even consider using them.

You should change the Observer type to be a class rather than a structure. If you do that though, you will need to move the creating of the Observer instance to inside the loop, as the others have said.

Well, I have used structures in C quite a lot, and as far as I know, I understand them quite well. I'm not at all sure why a Class would be better in this case, though I recognize that there may well be good reasons for it.

Quote

Also, if you do define a structure like you have here though, at least do yourself a favor, and make it immutable />

Why would it matter? As it sits, can another Class 'reach in' to this class and change the Structure?

Thanks for the comments.

[CodingSup3rnatur@l-360]

You are really best off forgetting about your knowledge of structures in C, and C++. In VB.NET, things are more complex where structures and classes are concerned, and the two are far more disjoint. You can absolutely use structures in place of classes in this case, and still get a functionally correct program. However, the point is that structures add extra complexity that classes simply don't (in general, not just in this case), and therefore you'd want to avoid using structures when you don't need to/get any benefit from their usage.

In VB.NET, we have two categories of type: value types and reference types. Structures are value types, classes are reference types.

This has various implications.

The main implication being that the entire contents of structures are copied when they are assigned to a variable (or added to a list), or passed to a method. When using structures, we are working with the value itself, where as when we use classes, we are working with references to the object only, rather than the actual object itself. All sorts of problems can follow from this difference.

The most obvious problem with this also exists also in C. If the structure is sizable, this is going to incur performance penalties due to all the copying that's going on, just as in C. By using classes, you automatically avoid this problem. As such, Microsoft recommend structures be no larger than 16 bytes in the .NET world. Your structure is already well in excess of 16 bytes.

Further, when you use a structure where type Object, or an interface type is expected (variable assignment, method argument etc), the structure is copied, and converted to the expected reference type (boxed). Now the structure behaves like a reference type, and will no longer be copied on every assignment operation. Further, if you convert the structure back to the structure type, it is unboxed and behaves with value type semantics again. Keeping track of what copies you have got where, and how each copy is behaving, can cause confusion and real problems. Plus, boxing and unboxing incur a performance penalty when done a lot.

A few restrictions imposed by structures

• Structures cannot, by default, be assigned a null value, meaning they always have to have a definite value unless you take special measures to change this.
  
• Structures are sealed meaning if you want to extend the behavior of your structure via inheritance, or encapsulate common/general behavior and data into a base class, you are out of luck.
  
• You cannot add code to the default constructor of a structure, because it is illegal to define your own default constructor.
  
• You cannot initialize instance fields inline. Couple this with the fact that you cannot define the default constructor, you will be forced expose the fields publicly (breaking encapsulation, a massive OOP no-no), and have the calling code initialize them directly (as you have done in your code), leading to more tightly coupled code, and a more procedural style, where a more OOP style is preferred.

Further, if you need to change Observer from a structure to a class in the future as the code grows, for example, in order to overcome one of the restrictions of structures (a very real possibility as a program grows, if you have used a structure from the wrong reasons), that change will almost certainly be a (silently) breaking change, due to the different semantics structures and classes exhibit.

Also, as there is usually no visual difference between the code that uses a structure, and code that uses a class (for example, a method parameter) for someone looking at/modifying the code where there is a mix of classes and structures, it can be very difficult to follow, and extremely easy to make a mistake, due to the difference in semantics structures and classes have.

In fact, that point was even demonstrated in this thread, where lucky3 and _HAWK_ overlooked (and I overlooked it upon first read of the thread, I have to admit) the fact that you were using a structure, and therefore the behavior of the code was different to what we expected. .NET developers instinctively think in terms of classes, rather than structures.

Finally, structs are typically more difficult to implement correctly than classes. For example, to avoid boxing structures when comparing them with Equals(), structs should implement the IEquatable<T> interface.

Why should structures be immutable?

You are exposing Observer instances to calling classes. They can therefore access them (by calling loadObservers()), and change them.

Moving on, structures in .NET should be immutable to help avoid the potentially puzzling and unintuitive results you can get when using structures. It can be very easy to change a structure, but not get the results you want because you changed an unexpected copy of the structure. In the most simple case, look at this code:

Dim observers As New List(Of Observers.Observer)
Dim newObserver As New Observers.Observer()
newObserver.Name = "Jason"
observers.Add(newObserver)
Console.WriteLine("Name before change: {0}", observers(0).Name)
Dim retrievedObserver As Observers.Observer = observers(0)
retrievedObserver.Name = "John"
Console.WriteLine("Name after change: {0}", observers(0).Name)

The name of the copy of the observer in the list remains unchanged, perhaps unsurprisingly in this trivial example. The same problem exists if you retrieved one of the observers from the list, and passed it to a method for further processing. This makes losing data easier than it would be if you were using a class.

Similarly,

Dim observer1 As New Observer()
observer.Name = "Jason";
Dim observer2 As Observer = observer1
observer2.Name = "John";

Now you have two distinct copies of the structure, where many may expect there to be only one, with the name "John". Look at how there is no difference in syntax in the use of the Observer type, regardless if it's a structure, or a class. As the number of distinct copies grow (and that number can grow very quickly), so does the brain power required to understand the code.


Further, here is a good blog entry from Eric Lippert (Principle Developer on the C# compiler team) on the subject of mutable structures in C#.NET (the exact same thing applies to VB.NET). His closing sentence sums up the situation really />/>/>

The C# code in that blog entry converted to VB.NET:

Spoiler

Module Test
    Private ReadOnly m As New Mutable()
    Sub Main()
        Console.WriteLine(m.Mutate())
        Console.WriteLine(m.Mutate())
        Console.WriteLine(m.Mutate())
        Console.ReadKey(True)
    End Sub
End Module

Structure Mutable
    Private i As Integer
    Public Function Mutate() As Integer
        Me.i = Me.i + 1
        Return Me.i
    End Function
End Structure

What does the code print? If Mutable was a class, people would barely need to think about it; it would be intuitive. However, as it is a structure, it takes a bit more thought as structures just don't behave in an intuitive manner, and many still get the answer wrong.

An basic example in your code of this lack of intuitiveness is the way you add the observers to the list. Which one of these approaches is more intuitive to someone looking at the code;

1) Creating one instance of the observer, and changing its properties on every iteration, and relying on the value type semantics of structures to correctly add the distinct observers to the list.

2) Creating multiple instances of the observers (one per loop iteration), setting each one's properties, and explicitly adding each distinct instance to the list.

Number 2 most clearly demonstrates the intent of the code.

OOP Design

Structures in C are obviously used solely to group pieces of data together, and you pass those structures to functions that operate on the data. I would try to get out of that way of thinking completely when using VB.NET, or any fundamentally OOP language. That's a very procedural way of thinking (which is entirely understandable given your programming background).

Generally speaking, in the world of OOP, you want to be thinking more about having classes that encapsulate both data AND behavior. Having anemic classes or structures that contain only data, but no behavior that operates on that data, may suggest you aren't fully embracing the OOP paradigm, and therefore not seeing the full benefits of it. That's not a universal thing, but I think should be a general rule when you have groups of related data, and functions that operate on that data; that is, the data and behavior should be grouped together into a data type.

Also, make sure you keep your instance fields private, and pass data into the classes or structures via constructors and/or properties.

Conclusion

Structures should be used where you require the value type semantics they offer (which shouldn't be that often), and where you want to represent a small, conceptually single value (a point, for example) that will not need to be boxed and/or unboxed too often.

If you can justify specifically why you need to use a structure rather than a class (such reasons do definitely exist, no matter how sparse), and you understand the consequences, then that's absolutely fine. Otherwise, why burden and restrict yourself, and other people reading/working with your code, with the complexity and restrictions that structures introduce? The last thing we want is extra, unneeded complexity, right? Particularly in a language as high level as VB.NET.

This post has been edited by CodingSup3rnatur@l-360: 21 January 2013 - 08:44 AM

[lar3ry]

Wow! That's going to take me a while to digest, I think. Looks like I have a LOT of habits to break. I may be back with more questions.

Thanks a million for the detailed explanation and references!

[_HAWK_]

Yeah, I don't use them for all those reasons. Nice expo.

This post has been edited by _HAWK_: 21 November 2012 - 07:19 PM

[lar3ry]

Well, I am definitely struggling a bit with this class. Here's what Ive done so far, in converting it from a single class with a Structure, to two classes. I've included a few stubs for things I want to add later.

Option Explicit On
Option Strict On
Imports System.IO

Public Class Observers

    Private _obslist As New List(Of Observer)

    Public Property ObsList() As List(Of Observer)
        Get
            Return _obslist
        End Get
        Set(ByVal ObsList As List(Of Observer))
            _obslist = ObsList
        End Set
    End Property

    Public Sub New()
        If File.Exists("Observers.txt") Then
            Dim sr As New StreamReader("Observers.txt")
            Dim s() As String
            Do
                s = Split(sr.ReadLine(), ",")
                Dim obs As New Observer(s(0), s(1), s(2), s(3), s(4), s(5), s(5))
                _obslist.Add(obs)
            Loop Until sr.Peek = -1
        End If
    End Sub

    Public Sub addObserver(ByVal Ob As Observer)

    End Sub

    Public Sub DeleteObserver(ByVal ObsName as String)

    End Sub

    Private Sub saveObservers(ByVal ObsList As List(Of Observer))

    End Sub

End Class

Class Observer
    Private _name As String
    Private _obslat As Double
    Private _obslong As Double
    Private _obselev As Integer
    Private _utcoffset As Int16
    Private _obsdst As Boolean
    Private _obsemail As String

    Public Sub New(ByVal Name As String,
                   ByVal ObsLat As String,
                   ByVal ObsLong As String,
                   ByVal ObsElev As String,
                   ByVal UTCOffset As String,
                   ByVal ObsDST As String,
                   ByVal ObsEmail As String
                   )
        _name = Name.Trim
        _obslat = CDbl(ObsLat)
        _obslong = CDbl(ObsLong)
        _obselev = CInt(ObsElev)
        _utcoffset = CShort(UTCOffset)
        _obsdst = CBool(ObsDST.Trim = "Y", vbTrue, vbFalse))  's(5).Trim
        _obsemail = ObsEmail.Trim
    End Sub
End Class

I get an error on the line: Public Property ObsList() As List(Of Observer) (and on the Sub stubs),

BC30909: 'ObsList' cannot expose type 'Observer' outside the project through class 'Observers'.

I have absolutely no idea how to fix it.

Should I even be trying to get the List of Observer?