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

Aggregating 2 lists by matching UniqueIDs

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

Problem

This Macro takes my company's 2 main sources of all-client data (each a data table in a separate worksheet) and aggregates them into a 3rd list, by matching a unique account number (for clients that have an account number).

As always, I'm welcome to all feedback on any aspect of the Code and in particular, I'm interested in:

-
Readability: Ability for somebody who is not me to come in blind, and (relatively) easily figure out how the whole thing works and fix
some problem that's cropped up.

-
Robustness: Designing subs/functions to deal with variable cases and/or to reliably fail when given unintended arguments.

-
Reusability: Designing subs/functions/the entire project so they can be easily re-purposed for future projects.

Workbook download if desired

N.B. As this is for internal consumption, I've designed the macro to take any size of data in any order, so long as the required Column Headings are present.

Module 1: "Combine_ACT_Ascentric_Data" - All Macros unique to this project

```
Option Explicit
Option Compare Text

Public wbCurrent As Workbook

Public wsAscentric As Worksheet
Public wsAct As Worksheet
Public wsCombinedList As Worksheet

Public arrCombinedData As Variant
Public lngCurrentRow As Variant

Public arrActData As Variant
Public arrAscentricData As Variant

Public colAscentricHeadings As Collection '/ Required Headings in the respective Data Sets
Public colActHeadings As Collection '/

Public Sub CombineACTandAscentricData()

Call StoreApplicationSettings
Call DisableApplicationSettings

'/======================================================================================================================================================
'/ Author: Zak Armstrong
'/ Email: zak.armstrong@luminwealth.co.uk
'/ Date: 25/August/2015
'/
'/ Description: Given the "All Client Wrappers" Data table from Ascentric and an Excel Export of ACT Client data, assign the des

Solution

Here are a couple of points about your standard subs & functions:

  • I've mentioned in another post that IsWorkbookOpen has a side-affect of activating the workbook.



  • At first glance the FindStringInRange doesn't seem to do anything, it's a sub, but then you realise that it changes the ByRef rngFoundCell Range variable. I think this should be a function which returns a Range object rather than altering a parameter. Much easier to see what is happening.



-
You are using variants for the application settings but why not use a boolean or the actual enum definition. That way you get intelli-sense help.

Public blnScreenUpdating    As Boolean
Public blnEnableEvents      As Boolean
Public varCalculation       As xlCalculation


-
In some places you have code that says something like If bBookIsOpen = False Then. Consider whether If Not bBookIsOpen Then is easier to understand. The naming of the variable makes a lot of difference here.

-
You've got several procs CopyArrayContentsXd that do a lot of similar things. Maybe consider merging this into a generic proc that takes a parameter for numDimensions or works it out from the array.

-
In PutSheetDataInArray you've got several Optional parameters with a default value of 10 (Why 10? Why not just leave empty?) However, you then use the IsMissing function on all of these parameters. On first glance, this makes no sense because how can a variable be missing if you've given it a default value? Looking at the help for the IsMissing function further reveals that it only works on Variants as well. You need to re-work this proc.

-
If you are using lots of arrays/collections and then testing if a value exists, consider whether a dictionary is a better choice.

That's all for now.

Code Snippets

Public blnScreenUpdating    As Boolean
Public blnEnableEvents      As Boolean
Public varCalculation       As xlCalculation

Context

StackExchange Code Review Q#101902, answer score: 5

Revisions (0)

No revisions yet.