[RESOLVED] Constructor Style

[dbasnett]

The Missouri Legislature is organized into general assemblies and sessions within a general assembly.

I am creating a general assembly object. The constructor(s) should be able to accept a number for the general assembly, or a year.

Currently I have one constructor. If the number is less than 1821 it is interpreted as a general assembly number. If it is greater than or equal to 1821 it is interpreted as a year.

The constructor looks like this

    ''' <summary>
    ''' general assembly
    ''' </summary>
    ''' <param name="NumberGAorYear"> if .lt. 1821 number is GA, else year</param>
    ''' <remarks>the 1820 GA occurs in year 5459</remarks>
    Public Sub New(NumberGAorYear As Integer)

Seeking opinions please.

[PlausiblyDamp]

The Missouri Legislature is organized into general assemblies and sessions within a general assembly.

I am creating a general assembly object. The constructor(s) should be able to accept a number for the general assembly, or a year.

Currently I have one constructor. If the number is less than 1821 it is interpreted as a general assembly number. If it is greater than or equal to 1821 it is interpreted as a year.

The constructor looks like this

    ''' <summary>
    ''' general assembly
    ''' </summary>
    ''' <param name="NumberGAorYear"> if .lt. 1821 number is GA, else year</param>
    ''' <remarks>the 1820 GA occurs in year 5459</remarks>
    Public Sub New(NumberGAorYear As Integer)

Seeking opinions please.

Personally I wouldn't go with this - it is going to be far too easy to miss this when maintaining your code, plus it isn't obvious when reading the code exactly what is going on.

How is this handled internally by the class? i.e. how does this parameter get used internally?

[dbasnett]

Personally I wouldn't go with this - it is going to be far too easy to miss this when maintaining your code, plus it isn't obvious when reading the code exactly what is going on.

How is this handled internally by the class? i.e. how does this parameter get used internally?

That is why I asked. Internally a check is made of NumberGAorYear.

It must be greater than zero.
If it is less than 1821 then it is a general assembly number. The year is calculated by multiplying it by 2 and adding that to 1819.
If it is equal to or greater than 1821 it is a year. The general assembly is calculated by subtracting 1819 from it and dividing by 2.

Each general assembly spans two years. Missouri's first general assembly spanned 1821 - 1822.

FWIW - the 1820 GA starts in year 5459.

[PlausiblyDamp]

I would do something like the following

    Sub Main(args As String())

        Dim tmp1 As GA = GA.FromYear(2000)
        Debug.WriteLine($"GA {tmp1.GANumber}, year {tmp1.Year}")

        Dim tmp2 As GA = GA.FromGANumber(2)
        Debug.WriteLine($"GA {tmp2.GANumber}, year {tmp2.Year}")
    End Sub
End Module

Class GA

    Private Sub New()

    End Sub
    Public Shared Function FromYear(year As Integer) As GA

        'do validation for year....
        Dim g As New GA With {
            .Year = year
        }
        Return g
    End Function

    Public Shared Function FromGANumber(number As Integer) As GA
        'do GA number validation here...
        Dim g As New GA With {
       .Year = (number * 2) + 1819
   }
        Return g
    End Function


    Private _year As Integer

    Public Property Year As Integer
        Private Set(value As Integer)
            _year = value
        End Set
        Get
            Return _year
        End Get
    End Property

    Public ReadOnly Property GANumber As Integer
        Get
            Return (_year - 1819) / 2
        End Get
    End Property
End Class

The Private Sub New prevents anyone from creating an instance directly, they need to use the FromYear / FromGANumber shared methods. Internally the class tracks the year (no reason, could just as easily been GA number) and exposes properties for GANumber and Year so the calcualtion is handled by the class.

I haven't included any validation for the year or GA number when creating the instance - this is just to give you an idea.

Also the Class name GA is pretty naff in my opinion as well, sure you could come up with a more suitable name!

[dbasnett]

I would do something like the following

    Sub Main(args As String())

        Dim tmp1 As GA = GA.FromYear(2000)
        Debug.WriteLine($"GA {tmp1.GANumber}, year {tmp1.Year}")

        Dim tmp2 As GA = GA.FromGANumber(2)
        Debug.WriteLine($"GA {tmp2.GANumber}, year {tmp2.Year}")
    End Sub
End Module

Class GA

    Private Sub New()

    End Sub
    Public Shared Function FromYear(year As Integer) As GA

        'do validation for year....
        Dim g As New GA With {
            .Year = year
        }
        Return g
    End Function

    Public Shared Function FromGANumber(number As Integer) As GA
        'do GA number validation here...
        Dim g As New GA With {
       .Year = (number * 2) + 1819
   }
        Return g
    End Function


    Private _year As Integer

    Public Property Year As Integer
        Private Set(value As Integer)
            _year = value
        End Set
        Get
            Return _year
        End Get
    End Property

    Public ReadOnly Property GANumber As Integer
        Get
            Return (_year - 1819) / 2
        End Get
    End Property
End Class

The Private Sub New prevents anyone from creating an instance directly, they need to use the FromYear / FromGANumber shared methods. Internally the class tracks the year (no reason, could just as easily been GA number) and exposes properties for GANumber and Year so the calcualtion is handled by the class.

I haven't included any validation for the year or GA number when creating the instance - this is just to give you an idea.

Also the Class name GA is pretty naff in my opinion as well, sure you could come up with a more suitable name!

I don't hate it. IMO the object creation is obscure but maybe it is just a matter of education. At least it gives me something to show those that will be using it.

THX!

[dbasnett]

After speaking with the others using this they came up with a third alternate. Don't hate this either, might be my favorite.

    Class GA
        ''' <summary>
        ''' 
        ''' </summary>
        ''' <param name="Year">year or Nothing</param>
        ''' <param name="GANum">GA number</param>
        ''' <remarks></remarks>
        Public Sub New(Optional Year As Integer? = Nothing,
                        Optional GANum As Integer? = Nothing)

            If Year.HasValue Then
                Stop
            ElseIf GANum.HasValue Then
                Stop
            Else
                Stop
            End If

        End Sub
    End Class

[techgnome]

Eww.... that;'s even worse.... I'd go with the factory methods personally... the provide clear, consice methods so you know EXACTLY what you're passing in and what you're getting back.


Using the thrid option there.... what happens when the user just uses the new method and doesn't supply any numbers? Or what if they supply both? What if one or both are 0? At least with factory methods, if the GetByYear is used, you KNOW it's a year. You can validate that and not worry about the GA number. Same for the getByGANumber ...

-tg

[PlausiblyDamp]

Personally I tend to prefer the factory methods for a couple of reasons...

Firstly, I find them a lot more obvious when reading the code.
Secondly, although it is legal for a constructor to throw am exception, it never seems quite right; a method that throws an exception seems a lot more natural.

[dbasnett]

Personally I tend to prefer the factory methods for a couple of reasons...

Firstly, I find them a lot more obvious when reading the code.
Secondly, although it is legal for a constructor to throw am exception, it never seems quite right; a method that throws an exception seems a lot more natural.

I am going with the factory methods. I've attached the output from my XML document processor, the help file if you will. Thanks for the help.