HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Growing my own Tree (Structure)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
owngrowingstructuretree

Problem

Summary

VB6 doesn't have a great selection of data structures to work with, so again I find myself creating my own. I have a need to dynamically generate a directory structure on the file system. The natural way to represent this is with a Tree, so I created my own.

There are three classes involved here:

  • TreeNode - The heart and soul of the data structure.



  • TreeNodes - A custom collection wrapper to hold nodes. This is iterable and is mainly used as a place to hold all of a TreeNode's child nodes.



  • Tree - this class' only responsibility is to hold the root node of a Tree.



I considered that Tree may be kind of useless, but having a separate class to hold the root node greatly simplified having to keep track of whether or not a TreeNode was a root or not. I'm not sure this was the right decision. I didn't really look at other tree implementations prior to designing this. I wanted to see if I could "get it right" on my own.

Other Concerns:

  • Have I missed any potentially useful functionality that would be expected out of a Tree?



  • I used a collection as the internal datatype for TreeNodes. Would a dictionary have been better? If so, how?



  • Are the documentation attributes helpful?



  • Do I have reasonable test coverage here? Did I miss any edge cases?



  • Did I do anything dirty? It's all on topic, including the unit tests.



Tree

VERSION 1.0 CLASS
BEGIN
MultiUse = -1 'True
END
Attribute VB_Name = "Tree"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = False
Attribute VB_Exposed = False
Option Explicit

Private Type TTree
Root As TreeNode
End Type

Private this As TTree

Public Property Get Root() As TreeNode
Set Root = this.Root
End Property

Public Property Set Root(ByVal Value As TreeNode)
Set this.Root = Value
End Property

Private Sub Class_Initialize()
Set this.Root = New TreeNode
End Sub


TreeNode

`VERSION 1.0 CLASS
BEGIN
MultiUse = -1 'True
END
Attribute

Solution

Public Property Set Root(ByVal Value As TreeNode)
    Set this.Root = Value
End Property

Private Sub Class_Initialize()
    Set this.Root = New TreeNode
End Sub


Why do you need to expose a setter at all? As soon as you create a new Tree, you're ready to add child nodes using myTree.Root.Children.

This brings me to this superfluous test here:

'@TestMethod
Public Sub RootIsNotNothingAfterSetting()
    'Arrange:
    Set t = New Tree

    'Act:
    Set t.Root = New TreeNode

    'Assert
    Assert.IsNotNothing t.Root
End Sub


You're already testing that Root is set upon tree creation; what the setter is allowing, really, is for weirdness like this:

Set myTree.Root = Nothing


Which defeats the test and highlights that your data structure is missing an important [tiny little] detail: immutability!

No one should ever be allowed to swap that Root reference! If client code needs a new Root, then they need a new Tree!

Public Sub ParentIsNotNothingAfterRemovingChild() 'TODO: Rename test


That test was definitely renamed, and yet you still have a TODO item here that could be removed.

'@TestInitialize
Public Sub TestInitialize()
    Set t = New Tree
    t.Root.Name = "C:"
End Sub

'@TestCleanup
Public Sub TestCleanup()
    Set t = Nothing
End Sub


The Rubberduck setup & teardown methods don't need @TestInitialize and @TestCleanup markers if they're named [respectively] TestInitialize and TestCleanup - I only know because I wrote the framework though :)

These markers exist in the event where one would like to use different names for setup & teardown. Note that this is also the reason why TestInitialize and TestCleanup cannot be used as test method names without a @TestMethod marker*.

* actually that's not exactly true - see issue #329 - "@TestMethod" marker has no effect on a method named "TestInitialize" or "TestCleanup"

Code Snippets

Public Property Set Root(ByVal Value As TreeNode)
    Set this.Root = Value
End Property

Private Sub Class_Initialize()
    Set this.Root = New TreeNode
End Sub
'@TestMethod
Public Sub RootIsNotNothingAfterSetting()
    'Arrange:
    Set t = New Tree

    'Act:
    Set t.Root = New TreeNode

    'Assert
    Assert.IsNotNothing t.Root
End Sub
Set myTree.Root = Nothing
Public Sub ParentIsNotNothingAfterRemovingChild() 'TODO: Rename test
'@TestInitialize
Public Sub TestInitialize()
    Set t = New Tree
    t.Root.Name = "C:"
End Sub

'@TestCleanup
Public Sub TestCleanup()
    Set t = Nothing
End Sub

Context

StackExchange Code Review Q#84575, answer score: 4

Revisions (0)

No revisions yet.