2

All,

I have a VB6 project with about 100 custom collection classes which I want to convert to VB.Net. A typical example would be something like.

Class CAccounts
     Private m_Accounts As New Collection

     Public Sub Add(newItem As CAccount)
         m_Accounts.Add newItem, newItem.IdKey
     End Sub
     Public Sub Remove(index As Variant)
         m_Accounts.Remove index
     End Sub
     Public Function Item(index As Variant) As CAccount
         Set Item = Nothing
         On Error Resume Next
         Set Item = m_Accounts.Item(index)
     End Function

     .........

Where the item being stored is

Class CAccount
    Public Id as long
    Public Code as String
    Public Name as string
    Public Sub Init(ByVal Id as Long)
        Me.Id = Id
        Code = ""
        Name = ""
    End Sub
    Public Property Get IdKey() as String
        IdKey = Code
    End Property

    ......

All of the collection classes in the project use this standard approach. However, not all the properties/methods of the collection classes are actually used. Most of the collection are used in "for each" loops. Keyed access using the string key is quite common. Keyed access by index is much less common.

Ideally I'd like to take a standard approach to converting these classes. I don't really want to have to review each collection and it's usage to consider whether I need a List, Dictionary, etc. Some of these collection contain 100,000 objects, and some will only contain 10. However, on the other hand I don't want to cause performance problems by using a more complex structure where a simpler option would do.

I'd appreciate any advice on the best approach. I've considered the following.

  1. Sticking with the old style Collection. So, it would be relatively easy to convert to VB.Net But, I'd rather move to the more modern structures.

  2. Have CAccounts Inherit KeyedCollection(Of String, CAccount). Fortunately most of the classes held in the collections do have the key as part of the class (eg CAccount.IdKey above). This seems to work well. However, relatively few classes will access the colelction by numeric index. So, perhaps this is overkill if I only want keyed access by the string key?

  3. Have CAccounts Inherit Dictionary(Of String, CAccount) for the classes where I don't need access by numeric index. The problem I have with this is that all the existing "for each" loops are like "for each account in accounts". I don't want to have to change all these occurences to something like "for each account in accounts.Values". Although perhaps I can get round this by changing the default property?

  4. Have CAccounts Inherit MyCollection(Of String, CAccount), where MyCollection is my own bespoke collection. This seems a bit too much hard work.

I'm inclined to go for option 2 - make everything a KeyedCollection. Then make exceptions if required (eg non-unique keys). However, if KeyedCollections are much slower than Dictionarys I might go for Dictionary as the standard, and just use KeyedCollection where I need access by numeric index.

All advice appreciated.

djk
  • 33
  • 5
  • With 100 custom classes seems like a big VB6 app. There will always be debate around the best way to upgrade, here is a recently closed question that discusses the pro's and con's of various methods that you may wish to consider: http://stackoverflow.com/questions/10252998/vb6-code-upgrade/10256670#comment13187430_10256670 – Jeremy Thompson Apr 22 '12 at 04:23
  • We've spent months if not years thinks about how to upgrade this app - we've tried out most of the conversion tools; run pilot projects; etc. etc. We've decided to use the free Microsoft tools to do the conversion, and then tidy up the output (Collections to generics, ADOBD to ADO, old .ocx grids to Datagrid, etc.). We realise this is a lot of work. But much of the business logic, and almost all of the data access is within classes which has a standard approach. So we have written code to global replace Recordsets to DataReaders, etc. Starting from scratch was not a realistic option. – djk Apr 22 '12 at 17:29

2 Answers2

1
  • Convert all collections to Dictionary(Of String, Whatever) or KeyedCollection, whichever seems more appropriate.
  • See if there are performance problems by running the program.
  • If there are, use a profiler to pinpoint the culprit and change its type to something more appropriate.

But in general I would always suggest a complete rewrite instead of trying to port VB6 code. In my experience, and from what I’ve heard from other developers, this will always consume less time, at least for code that’s being actively developed. If you have a write-once port where the codebase will never be touched again, porting may make more sense but then I’d not worry about using modern collections, just use the compatibility classes.

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • +1 but be very careful on rewriting - usually a partial port is much more sane. Here's what I did (http://stackoverflow.com/questions/4953725/contain-a-vb6-form-in-a-net-mdi 2nd comment in answer) and it took 18 months in 3 releases to do a VB6 app that had lived for 8 yrs. Also Joel said http://www.joelonsoftware.com/articles/fog0000000069.html a while back: The single worst strategic mistake that any software company can make [is to] decide to rewrite the code from scratch. – Jeremy Thompson Apr 22 '12 at 04:15
  • As mentioned above, we have gone for a partial port. I'm sure there are apps where a complete re-write is the correct approach. In our case, much of the business logic is algorithms constructed using relatively standard code in classes. This migrates pretty well. There are some very clumsy forms - we will re-write these. Our goal is to get code which is pretty close to what we would have done from scratch. – djk Apr 22 '12 at 17:38
  • @djk Kudos. It sounds like you have a consistent plan and know what you’re talking about. Best of success. – Konrad Rudolph Apr 22 '12 at 17:42
  • If I use a Dictionary, is there a way to keep the syntax "for each accounts in accounts", rather than "for each account in accounts.Values"? Perhaps by adding a new method to the Dictionary class CAccounts? If I can't do this easily, I'm think I'll go for the KeyedCollection - then I won't have to change the "for each" statements. – djk Apr 22 '12 at 17:44
  • @djk Yes if you want this syntax then use the `KeyedCollection`. Everything else is convoluted since the `Dictionary` class implements enumeration for `KeyValuePair(Of TKey, TValue)` and nothing you could do can change that. – Konrad Rudolph Apr 22 '12 at 17:48
  • Thanks. The one part of the approach I'm really struggling with is testing as I go along. I don't want to wait until everything is converted before starting testing - what if we've made some bad assumption? I've tried adding the converted VB.Net modules into a COM library, and changed all the properties/methods of old VB6 classes to call the new VB.Net classes. The problem is where the VB.Net class references an unconverted VB6 class, which then references a converted VB.Net class. I'm spending more time building test code than actually converting. But, that's another topic. – djk Apr 22 '12 at 17:54
0

You're going to want a KeyedCollection<TKey, TItem> MSDN article here: http://msdn.microsoft.com/en-us/library/ms132438.aspx

Imports System
Imports System.Collections.Generic
Imports System.Collections.ObjectModel

' This class represents a very simple keyed list of OrderItems,
' inheriting most of its behavior from the KeyedCollection and 
' Collection classes. The immediate base class is the constructed
' type KeyedCollection(Of Integer, OrderItem). When you inherit
' from KeyedCollection, the second generic type argument is the 
' type that you want to store in the collection -- in this case
' OrderItem. The first generic argument is the type that you want
' to use as a key. Its values must be calculated from OrderItem; 
' in this case it is the Integer field PartNumber, so SimpleOrder
' inherits KeyedCollection(Of Integer, OrderItem).
'
Public Class SimpleOrder
    Inherits KeyedCollection(Of Integer, OrderItem)

    ' The parameterless constructor of the base class creates a 
    ' KeyedCollection with an internal dictionary. For this code 
    ' example, no other constructors are exposed.
    '
    Public Sub New()
        MyBase.New()
    End Sub

    ' This is the only method that absolutely must be overridden,
    ' because without it the KeyedCollection cannot extract the
    ' keys from the items. The input parameter type is the 
    ' second generic type argument, in this case OrderItem, and 
    ' the return value type is the first generic type argument,
    ' in this case Integer.
    '
    Protected Overrides Function GetKeyForItem( _
        ByVal item As OrderItem) As Integer

        ' In this example, the key is the part number.
        Return item.PartNumber   
    End Function

End Class
Chris Gessler
  • 22,727
  • 7
  • 57
  • 83
  • Thanks. This looks like the way ahead. Do I need to include the constructor New()? If I feel brave, I might try starting with Inherits List(Of CAccount) for each collction class, and then see if the compiler identifies the use of any keyed access. If so, I'll change to a KeyedCollection. I'm going to forget about Dictionary because I can't keep the existing syntax "for each account in accounts". – djk Apr 22 '12 at 17:59
  • Including the New constructor is probably optional. List(Of CAccount) will not give you keyed collection, so you'll end up using Find() alot. – Chris Gessler Apr 22 '12 at 19:01
  • I would have liked to mark both answers as correct. Also, unfortunately, as a newcomer I don't have enough reputation to upvote your answer. But I appreciate the help. Thanks – djk Apr 25 '12 at 10:33