[doveraudio]

Hello disco citizens. I'm trying to get a clear understanding of object oriented design principles, and my main interest is in generics and abstraction. The problem was always "This" until all of a sudden my school is sending me to vb.net class next semester. So i had to deal with "Me". Well, long story short, i think I'm starting to understand it. I'll post some code, it's fairly simple, a base class creates a concept, then the next one provides some functionality, which is then utilized by the Gui. There's two implementations of the base class, and very sparse ones at that.

First the Base Class, chanceObject.vb

Public MustInherit Class chanceObject
    Public Property Value As Integer
    Public Property stringValue As String
    Public Overrides Function ToString() As String
        Return Me.stringValue
    End Function
    Public MustOverride Sub assignValue()
End Class

Then the implementation i did first, coin.vb

Public Class coin
    Inherits chanceObject
    Public Property boolValue As Boolean = False
    Overridable Function doFlip()
        Static x As New System.Random
        Me.Value = x.Next(0, 2)
        assignValue()
        Return Me
    End Function
    Public Overrides Sub assignValue()
        Select Case Me.Value
            Case 0
                stringValue = "Heads"
                boolValue = False
            Case 1
                stringValue = "Tails"
                boolValue = True
        End Select
    End Sub
End Class

And a die object was an obvious choice to complement

Public Class die
    Inherits chanceObject
    Public Property Sides As Integer
    Overridable Function doRoll()
        Static x As New System.Random
        Me.Value = x.Next(1, Sides + 1)
        assignValue()
        Return Me
    End Function
    Public Overrides Sub assignValue()
        stringValue = Me.Value
    End Sub
End Class

If you throw the code in a forms application, put a button on the form, and a multiline textbox, with a vertical scrollbar in it.
then use this code to demo the classes

Public Class Form1

    Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
        Me.TextBox1.Text = ""
        Dim myCoin As New coin
        Dim myDie As New die
        myDie.Sides = 6
        For i = 0 To 25
            myDie.doRoll()
            myCoin.doFlip()
            Me.TextBox1.Text += "Coin Result : " _
               & vbCrLf & myCoin.ToString & vbCrLf
            Me.TextBox1.Text += "Roll Result : " _
               & vbCrLf & myDie.ToString & vbCrLf
        Next
    End Sub

End Class

My question is what is right and wrong in the above code? how can i improve on it? A big thanks to AdamSpeight2008 for the overrides ToString thing, that was really the key i needed to see a "Pathway" for a chanceObject hierarchy!

This post has been edited by doveraudio: 28 December 2012 - 07:00 PM

[_HAWK_]

First thing I see:

The function doRoll should be a Sub as you don't really return 'Me'. And you don't need the assignValue Sub just set the value. Take out the extra fluff when you can. KISS principle.

Public Class die
    Inherits chanceObject
    Public Property Sides As Integer
    Overridable Function doRoll()
        Static x As New System.Random
        Me.Value = x.Next(1, Sides + 1)
        assignValue()
        Return Me
    End Function
    Public Overrides Sub assignValue()
        stringValue = Me.Value
    End Sub
End Class

[doveraudio]

Ok, cool.

I threw myself into further exploration. Suppose i wanted to make a type of "die" that returned a word, instead of just blindly casting in the integer. I thought, first, I'd want to fix the number of sides, but i think i did a crap job on that. there's got to be a "right way"

Public Class dSix
    Inherits die
    Public Overrides Property Sides As Integer
        Get
            Return 6
        End Get
        Set(value As Integer)
            MyBase.Sides = 6
        End Set
    End Property
    Public Overrides Sub assignValue()
        Select Case Me.Value
            Case 1
                Me.stringValue = "One"
            Case 2
                Me.stringValue = "Two"
            Case 3
                Me.stringValue = "Three"
            Case 4
                Me.stringValue = "Four"
            Case 5
                Me.stringValue = "Five"
            Case 6
                Me.stringValue = "Six"
        End Select
    End Sub
End Class

I tried to use the auto thing that it threw in, but i don't think i did it right. I kind of just threw a potato at it.

This post has been edited by doveraudio: 28 December 2012 - 07:59 PM

[_HAWK_]

I didn't have a problem with a multi-sided die. I remember the good ol' days of 20 sided die. I like to make user friendly objects. What properties should the object have - like if someone was exploring it would it have the things they would expect? If you want to give an option for number of sides you can use the constructor to set it, but if they don't you need a backup plan - like if they use the default constructor and don't a set a number - so the program does not throw snake-eyes at you.

[doveraudio]

after i tried that out, i decided that was sufficient to "Hard code it", if it gets set to anything, it actually doesn't... ? I tried just setting it in one line, but then assignments worked. Not sure if it really MATTERS in this instance, but it's a good thing to know in the future. What to do when you don't want child objects setting a value which higher up in the hierarchy is available for manipulation? In this instance suppose i wanted to make a set of dSix, dEight die, etc, and they would have pictures and sounds determined by the Select Case. What exactly is proper throughout oop-dom to ensure that by no means could any further inheritance past that point overwrite that "Sides" Variable?

[CodingSup3rnatur@l-360]

Quote

Take out the extra fluff when you can. KISS principle.

Absolutely!

doveraudio, I'll just go through some quick observations I made about your code.

Encapsulation

Firstly, I think you are exposing too much to the outside world from your classes, and therefore you're breaking the OOP principle of encapsulation. Two examples are:

1)

Public MustOverride Sub assignValue()

Should the caller be able to call this method? Exposing this is also leaking the abstraction provided by the chanceObject class, and all it's descendants, when you consider the way that method is used in the derived classes.

2)

Public Property boolValue As Boolean = False

Should the caller be able to modify this value directly?

Naming Conventions

Secondly, you are breaking some of the naming conventions of VB.NET. For example, class names, and method names should use Pascal Casing. See here for general naming guidelines.

Inheritance

The thing about inheritance is that it should be used very sparingly. It is a very powerful technique, that, on certain occasions, can really improve code, and minimize its complexity. However, when abused, it can create horrible code that's hell to maintain.

Obviously, a 'Die' 'IS-A' 'ChanceObject'. However, I don't think the presence of the 'IS-A' relationship always indicates a need for inheritance.

One of the key benefits of a good use of inheritance is that it allows the calling code to focus on the general concept defined by the base class, without worrying at all about what concrete type they are actual working with. I.e. The derived classes are completely substitutable when working with a variable of the base class type.

Are you really ever going to work with variables of type ChanceObject like this, for example:

Public Sub MyMethod(ByVal co As ChanceObject)
    'work with chance object without worrying whether it is a die, a coin, a magic eight ball etc
End Sub

If not, what are you really gaining from the use of inheritance? Do you really need it, or is it just adding unneeded complexity?

If you did happen to define a method like that, according to the Liskov Substitutability Principle, that method should not care at all whether the passed in chance object is a die, or a coin, and it should work exactly the same way regardless of whether the chance object is a coin or a die.

What about the Value property though, for example? Does that really mean the same thing for coins, and dice? For coins it is simply flag value that represents heads or tails, but for dice it represents an actual numeric value which can be used in arithmetic, and such like.

This subtle difference in semantics suggests to me that, at the very least, Value perhaps shouldn't be inherited in derived classes (and definitely shouldn't be mutable to callers), and makes me question the use of inheritance here.

The problem with that is that I can see it possibly leading to a situation where the writer of MyMethod() has to stop and think - "Is this object that I am working with a coin, or a die (or a magic eight ball, or a lottery ticket etc)?". At that point, the use of inheritance adds to the complexity of the code, and starts to become harmful.

Example without inheritance

Here are your Coin and Die classes written separately without inheritance, with a few modifications here and there:

Coin

Spoiler

Public Class Coin

    Public Enum CoinFace
        Heads = 0
        Tails = 1
    End Enum

    Private _face As CoinFace

    Public ReadOnly Property Face As CoinFace
        Get
            Return Me._face
        End Get
    End Property

    Public Sub New()
        Me._face = CoinFace.Heads
    End Sub

    Public Sub Toss(ByVal rng As Random)
        Me._face = CType(rng.Next(0, 2), CoinFace)
    End Sub

    Public Overrides Function ToString() As String
        Return Me.Face.ToString()
    End Function

End Class

Die

Spoiler

Public Class Die

    Private _faceValue As Integer
    Private ReadOnly _sides As Integer

    Public ReadOnly Property FaceValue As Integer
        Get
            Return Me._faceValue
        End Get
    End Property

    Public ReadOnly Property Sides As Integer
        Get
            Return Me._sides
        End Get
    End Property

    Public Sub New(ByVal sides As Integer)
        Me._sides = sides
        Me._faceValue = 1
    End Sub

    Public Sub New()
        Me.New(6)
    End Sub

    Public Sub Roll(ByVal rng As Random)
        Me._faceValue = rng.Next(1, Sides + 1)
    End Sub

    Public Overrides Function ToString() As String
        Return Me.FaceValue.ToString()
    End Function

End Class

(NOTE: I pass the Random object in to the Roll() and Flip() methods because I like making dependencies explicit, as it gives callers flexibility, makes class interactions clearer, and helps when testing the classes. You can decide whether that offers you any benefit in your particular situation />).

Obviously, the difference isn't massive with such a small amount of code, but I think my modified classes are still less complex, more understandable, and better encapsulated than yours.

Conclusion

My message is really just to think twice before jumping into inheritance. If you ultimately decide that the 'IS-A' relationship is there, and inheritance gives you genuine benefits (beyond simple reuse), and reductions in complexity, then by all means go ahead and use it. Just make sure that you only inherit members that are genuinely needed by the sub classes, and make sure inherited members are all the same in their semantic meaning, and the contract they present to callers, across ALL derived class (and base classes).

Quote

What exactly is proper throughout oop-dom to ensure that by no means could any further inheritance past that point overwrite that "Sides" Variable?

To do that, you can just remove the setter part of the property (or make the setter private), and make the _sides backing field ReadOnly:

Private ReadOnly _sides As Integer
Public ReadOnly Property Sides As Integer
    Get
        Return Me._sides
    End Get
End Property

and initialize _sides from the derived classes indirectly using a constructor in the base class:

Public Sub New()
    MyBase.New(6) 'call base class constructor, passing 6 as the number of sides. Cannot modify Sides after this call.
End New

This post has been edited by CodingSup3rnatur@l-360: 29 December 2012 - 01:34 PM

[doveraudio]

CodingSup3rnatur@l-360 , thanks! good post, i think i learned something. I do realize some the issues with what i did there with the chanceObject in actual practice. I'm just using it as an easy to implement base class by which to derive similar objects. The idea was that i was creating an object with flexibility, ie, in some cases i can use a coin for a boolean, or others add pictures to the object, blah blah. just an effort towards escaping procedural type of stuff. THank you for the advice in applied programming practice! very informative.

[doveraudio]

When I went back and examined the implementation and thought about it, and read up a bit on the things that you suggested about Oop principles, I feel like my "ChanceObject" is exactly what i would want it to be, at my level of understanding. Its a generic object at its base, which regardless of anything else, it requires that its successors invoke a method to overwrite the .toString, ensuring that the object can provide a message in the form of a string. In implementing, i was trying to come up with a way to practice creating objects which could work in various settings. In the final bit of code, I present a way in which to demonstrate 2 derived classes being used interchangeably, actually, in the way the previous poster there mentioned. I'm interested in low level objects and generics primarily, and although this object isn't very generic, it's a sort of abstract concept which can be further developed. I visualize this chance object as a wire frame with mutable dimension and datatypes, and a constant datatype of string. I also am using it to try and understand blank(Of t), as it would be a handy way to make a deck of cards, cup of dice, or whatever.

[CodingSup3rnatur@l-360]

Okay, that's absolutely fine.

I'll just throw a few points/examples your way...

I think your requirement that all subclasses should provide a string for display purposes could be implemented in a slightly clearer way. Perhaps something like:

Spoiler

Public MustInherit Class ChanceObject
    Private _value As Integer
    'A value that was generated by chance
    Public Property Value As Integer
        Get
            Return Me._value
        End Get
        Protected Set(ByVal value As Integer)
            Me._value = value
        End Set
    End Property

    Protected Sub New(ByVal startingValue As Integer)
        Me._value = startingValue
    End Sub

    'Generates a chance value. This chance value is assigned to Value.
    Protected MustOverride Sub GenerateValue() 
    Protected MustOverride Function InternalToString() As String

    Public NotOverridable Overrides Function ToString() As String
        Return Me.InternalToString()
    End Function

End Class

Now derived classes are forced to provide a string, and it is made crystal clear that this is a requirement of derived classes, and there is a clear distinction between the generation of the chance value, and it's string representation. I think this makes implementing derived classes that little bit easier.

You could also add the concept of chance to the base class, as, after all, it is modeling an object that is driven by chance, if you felt that would be beneficial, that is. Further, in the next example, as you mentioned generics in your last post, I'll make the data type of Value generic, just to show you a simple example of generics:

Spoiler

Public MustInherit Class ChanceObject(Of TValue)
    Private _value As TValue

    'A value that was generated by chance
    Public ReadOnly Property Value As TValue
        Get
            Return Me._value
        End Get
    End Property

    Protected Sub New(ByVal startingValue As TValue)
        Me._value = startingValue
    End Sub

    Public Sub Randomize(ByVal rng As Random)
        Me._value = Me.OnValueRequired(rng)
    End Sub

    'Returns a value generated by chance.
    Protected MustOverride Function OnValueRequired(ByVal rng As Random) As TValue
    Protected MustOverride Function InternalToString() As String

    Public NotOverridable Overrides Function ToString() As String
        Return Me.InternalToString()
    End Function

End Class

Public Class Coin
    Inherits ChanceObject(Of Boolean)

    Public Const HEADS As Boolean = True
    Public Const TAILS As Boolean = False

    Public Sub New()
        MyBase.New(HEADS)
    End Sub

    Protected Overrides Function InternalToString() As String
        Return If(Me.Value = HEADS, "Heads", "Tails")
    End Function

    Protected Overrides Function OnValueRequired(ByVal rng As Random) As Boolean
        Return rng.Next(0, 2) = 0
    End Function
End Class

Public Class Die
    Inherits ChanceObject(Of Integer)

    Private ReadOnly _sides As Integer
    Public ReadOnly Property Sides As Integer
        Get
            Return Me._sides
        End Get
    End Property

    Public Sub New(ByVal sides As Integer)
        MyBase.New(1)
        Me._sides = sides
    End Sub

    Public Sub New()
        Me.New(6)
    End Sub

    Protected Overrides Function InternalToString() As String
        Return Me.Value.ToWritten() 'See text below this code
    End Function

    Protected Overrides Function OnValueRequired(ByVal rng As Random) As Integer
        Return rng.Next(1, Me.Sides + 1)
    End Function
End Class

OnValueRequired() is a method derived classes must override in that example, and its contract states that it simply returns a chance/random value. With that implementation, derived classes don't have to/cannot worry about changing the value of Value, so the base class' encapsulation is improved. Writing derived classes becomes even easier as all they have to worry about is returning a value for those two abstract functions.


Finally, you can flatten (and generalize) that hard coded switch statement in the Die class using a utility method (or an extension method as I have done in the above code), that can convert integers to their written form, like is shown here, for example. If you have a chance to flatten switch statements, it's often a good idea.

Those very simple code snippets may not be what you are going for, but seeing as you seem to be on a learning pursuit, they may act as basic demonstrations on certain aspects of inheritance and generics