0

I have a class in the code below, where besides equals and hash methods from IEqualityComparer which I use, I also want to implement add, remove, item, count from list and GetEnumerator (current,movenext,position). So I decided to use Inherits CollectionBase, IEnumerator and IEnumerable.

Anyway for instance when I use Add its not going to Add method in Part class, or when I do for each its not going to GetEnumerator move next. What is the problem?

This is the class:

Imports System.Collections.Generic

Public Class Part
    Inherits CollectionBase
    Implements IEqualityComparer(Of Part), IEnumerator(Of Part), IEnumerable(Of Part)

    Private _values As List(Of Part)
    Private _currentIndex As Integer

    Public Property _comparisonType As EqualsComparmission

    Public Sub New(ComparisonType As EqualsComparmission)
        Me._comparisonType = ComparisonType
    End Sub

    Public Sub New(values As List(Of Part))
        _values = New List(Of Part)(values)
        Reset()
    End Sub

    Public Sub New()
    End Sub

    Public Property PartName() As String

    Public Property PartId() As Integer


    Public Overrides Function ToString() As String
        Return "ID: " & PartId & "   Name: " & PartName
    End Function


    Public Sub Add(ByVal value As Part)
        Me.List.Add(value)
    End Sub

    Public Sub Remove(ByVal Index As Integer)
        If Index >= 0 And Index < Count Then
            List.Remove(Index)
        End If
    End Sub

    Public ReadOnly Property Item(ByVal Index As Integer) As Part
        Get
            Return CType(List.Item(Index), Part)
        End Get
    End Property

    Public ReadOnly Property Current() As Part
        Get
            Return _values(_currentIndex)
        End Get
    End Property

    Public ReadOnly Property Current1 As Object Implements IEnumerator.Current
        Get
            Return Current
        End Get
    End Property

    Public Function MoveNext() As Boolean Implements IEnumerator.MoveNext
        _currentIndex += 1
        Return _currentIndex < _values.Count
    End Function

    Public Sub Reset() Implements IEnumerator.Reset
        _currentIndex = -1
    End Sub

#Region "IDisposable Support"
    Private disposedValue As Boolean ' To detect redundant calls

    ' IDisposable
    Protected Overridable Sub Dispose(disposing As Boolean)
        If Not Me.disposedValue Then
            If disposing Then
                ' TODO: dispose managed state (managed objects).
            End If

            ' TODO: free unmanaged resources (unmanaged objects) and override Finalize() below.
            ' TODO: set large fields to null.
        End If
        Me.disposedValue = True
    End Sub

    ' TODO: override Finalize() only if Dispose(ByVal disposing As Boolean) above has code to free unmanaged resources.
    'Protected Overrides Sub Finalize()
    '    ' Do not change this code.  Put cleanup code in Dispose(ByVal disposing As Boolean) above.
    '    Dispose(False)
    '    MyBase.Finalize()
    'End Sub

    ' This code added by Visual Basic to correctly implement the disposable pattern.
    Public Sub Dispose() Implements IDisposable.Dispose
        ' Do not change this code.  Put cleanup code in Dispose(disposing As Boolean) above.
        Dispose(True)
        GC.SuppressFinalize(Me)
    End Sub
#End Region

    Public Function GetEnumerator1() As IEnumerator(Of Part) Implements IEnumerable(Of Part).GetEnumerator
        Return CType(Me, IEnumerator)
    End Function

    Public ReadOnly Property Current2 As Part Implements IEnumerator(Of Part).Current
        Get
            Return Current
        End Get
    End Property

    Public Function Equals1(x As Part, y As Part) As Boolean Implements System.Collections.Generic.IEqualityComparer(Of Part).Equals
        If x Is Nothing AndAlso y Is Nothing Then Return True
        If x Is Nothing OrElse y Is Nothing Then Return False

        Select Case _comparisonType
            Case EqualsComparmission.PartId
                Return x.PartId = y.PartId
            Case EqualsComparmission.PartName
                Return String.Equals(x.PartName, y.PartName)
            Case EqualsComparmission.PartId_and_PartName
                Return x.PartId = y.PartId AndAlso String.Equals(x.PartName, y.PartName)
            Case Else
                Throw New NotSupportedException("Unknown comparison type for parts: " & _comparisonType.ToString())
        End Select
    End Function

    Public Function GetHashCode1(obj As Part) As Integer Implements System.Collections.Generic.IEqualityComparer(Of Part).GetHashCode
        Select Case _comparisonType
            Case EqualsComparmission.PartId
                Return obj.PartId
            Case EqualsComparmission.PartName
                Return If(obj.PartName Is Nothing, 0, obj.PartName.GetHashCode())
            Case EqualsComparmission.PartId_and_PartName
                Dim hash = 17

                hash = hash * 23 + obj.PartId
                hash = hash * 23 + If(obj.PartName Is Nothing, 0, obj.PartName.GetHashCode())
                Return hash
            Case Else
                Throw New NotSupportedException("Unknown comparison type for parts: " & _comparisonType.ToString())
        End Select
    End Function

End Class

and this is test code:

Dim parts As New List(Of Part)()

   parts.Add(New Part() With { _
           .PartName = "ala", _
           .PartId = 11 _
      })
        parts.Add(New Part() With { _
             .PartName = "shift lever", _
             .PartId = 1634 _
        })

   For Each aPart As Part In parts
            Console.WriteLine(aPart)
        Next

Edited:

PartsCollection which implements ICollection for list operations:

Public Class PartsCollection
    Implements ICollection(Of Part)

    ' Enumerators are positioned before the first element 
    ' until the first MoveNext() call. 
    Dim position As Integer = -1

    Private myList As List(Of Part)

    Public Sub New()

        If myList Is Nothing Then
            myList = New List(Of Part)
        End If

    End Sub

    Public Sub Add(item As Part) Implements ICollection(Of Part).Add
        myList.Add(item)
    End Sub

    Public Sub Clear() Implements ICollection(Of Part).Clear

    End Sub

    Public Function Contains1(item As Part) As Boolean Implements ICollection(Of Part).Contains
        Return myList.Contains(item)
    End Function

    Public Sub CopyTo(array() As Part, arrayIndex As Integer) Implements ICollection(Of Part).CopyTo

    End Sub

    Public Function GetEnumerator() As IEnumerator(Of Part) Implements IEnumerable(Of Part).GetEnumerator
        Return New PartsEnumeration(myList)
    End Function



    Public ReadOnly Property Count As Integer Implements ICollection(Of Part).Count
        Get

        End Get
    End Property

    Public ReadOnly Property IsReadOnly As Boolean Implements ICollection(Of Part).IsReadOnly
        Get

        End Get
    End Property

    Public Function Remove(item As Part) As Boolean Implements ICollection(Of Part).Remove

    End Function

    Public Function GetEnumerator1() As IEnumerator Implements IEnumerable.GetEnumerator

    End Function
End Class

PartsEnumeration for for each loop:

Public Class PartsEnumeration
    Implements IEnumerator(Of Part)

    Private mmyList As List(Of Part)
    Dim position As Integer = -1

    Public Sub New(ByVal myList As List(Of Part))
        mmyList = myList
    End Sub

    Public ReadOnly Property Current As Part Implements IEnumerator(Of Part).Current
        Get
            Try
                Return mmyList(position)
            Catch ex As IndexOutOfRangeException
                Throw New InvalidOperationException()
            End Try
        End Get
    End Property


    Public ReadOnly Property Current1 As Object Implements IEnumerator.Current
        Get
            Try
                Return mmyList(position)
            Catch ex As IndexOutOfRangeException
                Throw New InvalidOperationException()
            End Try
        End Get
    End Property

    Public Function MoveNext() As Boolean Implements IEnumerator.MoveNext
        If position < mmyList.Count - 1 Then
            position += 1
            Return True
        End If
        Return False
    End Function

    Public Sub Reset() Implements IEnumerator.Reset
        position = -1
    End Sub

#Region "IDisposable Support"
    Private disposedValue As Boolean ' To detect redundant calls

    ' IDisposable
    Protected Overridable Sub Dispose(disposing As Boolean)
        If Not Me.disposedValue Then
            If disposing Then
                ' TODO: dispose managed state (managed objects).
            End If

            ' TODO: free unmanaged resources (unmanaged objects) and override Finalize() below.
            ' TODO: set large fields to null.
        End If
        Me.disposedValue = True
    End Sub

    ' TODO: override Finalize() only if Dispose(ByVal disposing As Boolean) above has code to free unmanaged resources.
    'Protected Overrides Sub Finalize()
    '    ' Do not change this code.  Put cleanup code in Dispose(ByVal disposing As Boolean) above.
    '    Dispose(False)
    '    MyBase.Finalize()
    'End Sub

    ' This code added by Visual Basic to correctly implement the disposable pattern.
    Public Sub Dispose() Implements IDisposable.Dispose
        ' Do not change this code.  Put cleanup code in Dispose(disposing As Boolean) above.
        Dispose(True)
        GC.SuppressFinalize(Me)
    End Sub
#End Region

End Class

Test code:

 Dim cos As New Part With { _
             .PartName = "crank arm", _
             .PartId = 1234 _
        }

        Dim cos3 As New Part With { _
          .PartName = "cranddk arm", _
          .PartId = 123334 _
     }

        myParts.Add(something1)
        myParts.Add(something2)

        Dim p As Boolean = myParts.Contains1(something1)
        Console.WriteLine(p)

        For Each ii In myParts
            Console.WriteLine(ii.ToString)
        Next
unknown
  • 75
  • 7
  • 2
    You don't want Part to inherit CollectionBase. CollectionBase would be something you would inherit if you were implementing the List yourself. But List(Of T) already has all that functionality. – dwilliss Jan 08 '15 at 22:22
  • 1
    I would either inherit `Collection(Of T)` or simply implement a List. CollectionBase is a bit dated. The collection class would be something other than Part though. `Part` defines what goes in `Parts` which is a collection of Parts. – Ňɏssa Pøngjǣrdenlarp Jan 08 '15 at 22:24
  • i implemented ICollection and removed CollectionBase and its still not reaching objects... See my edit in first post – unknown Jan 08 '15 at 22:31
  • just move your existing parts List(of Part) into a new `Parts` class to manage the collection. That will work fine. Dont worry about interfaces or even inheritance until you get the basic functionality working. code like that proposed extension would go there. – Ňɏssa Pøngjǣrdenlarp Jan 08 '15 at 22:32
  • you mean to stay with class "Part" as i have just to remove CollectionBase and additionaly create new Class called "Parts" and Implements ICollection(Of Part) ? – unknown Jan 08 '15 at 22:41
  • `ICollection` makes Parts an actual collection. you do not need that PLUS a List(Of Part). get rid of ICollection. If/when you want to go in that direction, use `System.Collections.ObjectModel.Collection(of T)` Assuming you have gobs of code using parts as list(Part), start with that and simply move that code to the class. It will be perfectly functional. – Ňɏssa Pøngjǣrdenlarp Jan 08 '15 at 22:58
  • In the demo for the article I recommended, Zitems.vb creates 3 collections using 3 diff means: Collection, CollectionBase, and ObservableCollection. Starting with `ZItems`. `Collection` tends to be preferred because it will provide much of the functionality for you (add,Clear, Count...). – Ňɏssa Pøngjǣrdenlarp Jan 08 '15 at 23:14
  • still massive overkill - all those things ICollection requires are **already** implemented on `myList`. If you want Parts to BE a collection rather than consume one, it should inherit from Collection as I have said. – Ňɏssa Pøngjǣrdenlarp Jan 08 '15 at 23:22

1 Answers1

1

You are gluing 2 things together that are related but not the same thing and making a list of collections.

Dim parts As New List(Of Part)()

Since Part inherits from CollectionBase, you are making a List of Collections. A Collection class should be used to implement methods to manage the list/collection such as the Extension functionality in the other question. It would manage the List for you.

Class Part
    Property Name As String
    Property ID As Integer
End Class

Class Parts
    Private myList As New List(Of Part)

    Public Sub Add(item As Part)

    Public Function IndexOfPartName...

    etc

    Public Sub Remove(item As Part)

    ' some might just be wrappers:
    Public Function Contains(p As Part) As Boolean
        Return myList.Contains(Part)
    End Function

End Class

The code that consumes these:

Friend myParts As New Parts

myParts.Add(New Part(...))

Dim p As Part = myParts.IndexOfPartName("screws")

Once you work out the functionality you need and how it will be used, you might change it to Inherit Collection<T> so you can do For Each p As Part in Parts. But work on getting a viable collection class before tackling interfaces and such. Some of those may not be needed since Collection<T> implements all that for you.

You might be interested in: Guidelines for Collections

--

For your parts collection class to BE a collection rather then be a wrapper for an internal one:

Imports System.ComponentModel
Class Parts
    Inherits Collection(Of Part)

    ' ALREADY implemeted for you are:
    ' Contains, Count, Add, Clear, IndexOf, Insert, EQUALS
    ' Item, Items, Remove, RemoveAt and RemoveItem

As such, all you need to do is override those methods which do something differently than you would like, or to extend functionality such as a IndexOfPartName.

ICollection requires you to write a collection from scratch, but the wheel has already been built.

Ňɏssa Pøngjǣrdenlarp
  • 38,411
  • 12
  • 59
  • 178
  • check please my two classes within my first post in Edit clause strange – unknown Jan 08 '15 at 22:52
  • seems now its ok - see my Edit block again for Parts class. I dont know only why it is creating for me double time e.g GetEnumerator1 and GetEnumerator – unknown Jan 08 '15 at 23:19
  • I made two classes one for list playing "PartsCollection" and second for for each looping: "PartsEnumeration" i tested it and seems to be ok - see Edit in my main post. What do you think? I noticed thats its better to always seperate for each interface i am using otherwise diffrent issues occured. I still cant figure out why i get two times: GetEnumerator() and GetEnumerator1() – unknown Jan 08 '15 at 23:51
  • what you think is it the way it should be (separated classes)? – unknown Jan 09 '15 at 01:17
  • it must be 2 classes - you can have a Part be an Item and a collection of items. That was what was wrong with the first post - each Part was a collection of 1. BTW you do not need `IEnumerator` because both List and Collection implement it for you. See my last edit for all the stuff you DONT have to write. – Ňɏssa Pøngjǣrdenlarp Jan 09 '15 at 01:29
  • ok so i assume the way i've done it so far its ok, but what you are saying is i dont need PartsEnumeration class to inherit IEnumerator write? But if i will not inherit this interface then this class will not have any class so therefore having that class is not required and i should limit only to Part and PartsCollection is that correect? – unknown Jan 09 '15 at 01:35
  • that should read `you CAN'T have a part...` in my last comment. I am saying IEnumerator is not needed and ICollection is reinventing the wheel. All the normal collection functionality will be there using a class which inherits Collection. The MSDN link says the same thing. – Ňɏssa Pøngjǣrdenlarp Jan 09 '15 at 01:40
  • 1
    i see. I will stay only then with classes: Parts and PartsCollection it should be enough. – unknown Jan 09 '15 at 01:42
  • that demo I linked you to has LOTS of collections in it - some complex and nested, some simple – Ňɏssa Pøngjǣrdenlarp Jan 09 '15 at 01:44
  • just last question if i use like it is right now in Edited (first post): classes: Part and PartCollection it is good way of implementing according OOP and also if i would like to extend IEnumerator for some more complicated things i woudlc reate thirds class as i did and inherits IEnumerator? I just want to know if this technic is one of good way of doing in OOP way.,. – unknown Jan 09 '15 at 01:46
  • `For Each p As Part in Parts` is pretty complete as it is, and is free with List and Collection. For more complex, there is LINQ and lambda. IEnumerator is for classes you need to iterate which do not already have an enumerator, thats not the case here. – Ňɏssa Pøngjǣrdenlarp Jan 09 '15 at 01:52
  • i see. The only one problem i have now is when i try to Dim p As Boolean = myParts.Contains(New Part With { _ .PartName = "John", _ .PartId = 4 _ }) its going to PartsCollection class Contains1 method, but i would like it to go to Part class Equals1 method as i already defined it as i want to be compared there. – unknown Jan 09 '15 at 02:24
  • what i should do in this case? – unknown Jan 09 '15 at 02:41
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/68455/discussion-between-unknown-and-plutonix). – unknown Jan 09 '15 at 03:02