patternMinor
Aggregating 2 lists by matching UniqueIDs
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
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:
-
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.
-
In some places you have code that says something like
-
You've got several procs
-
In
-
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.
- I've mentioned in another post that
IsWorkbookOpenhas a side-affect of activating the workbook.
- At first glance the
FindStringInRangedoesn't seem to do anything, it's a sub, but then you realise that it changes theByRef rngFoundCell Rangevariable. I think this should be a function which returns aRangeobject 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 xlCalculationContext
StackExchange Code Review Q#101902, answer score: 5
Revisions (0)
No revisions yet.