I am looking to enhance my VBA skills and efficiency with the code below. It currently executes at about 20 minutes. I just recently learned how to use public functions but this project is very robust and I haven't been able to make it more efficient. I've cut off two Case requirements {Auto - Sentra/Van Fleet ($) ; Other ($)}
to be able to fit the code with its public functions. They repeat the same functionality expect the range input at the end of calling the functions changes. I'm assuming dictionaries will be a good starting point but I'm having difficulty comprehending its functionality.
Dim Wb1 As Workbook, Wb2 As Workbook
Dim Wk4 As Worksheet, Wk5 As Worksheet, Wk7 As Worksheet
Dim Wk1 As Worksheet
Dim CCRg As Range, CLinkRg As Range
Dim AllEntRg As Range, EntityOnlyRg As Range
Dim TypeRg As Range, GLRg As Range, OpsRg As Range
Dim FRow As Long, lRow As Long
Dim InlandRg As Range
Dim ATLNrg As Range, ATLErg As Range, ATLSrg As Range
Dim CCCodeRow As Long, CCCodeCol As Long
Sub Maverick()
starttime = Now()
Application.ScreenUpdating = False
Set Wb1 = Workbooks("SubModel Forecast_Other Admin v4.xlsm")
Set Wb2 = Workbooks("Feb15 PNL.xlsx")
Set Wk4 = Wb1.Sheets("ASSUMPTIONS")
Set Wk5 = Wb1.Sheets("Validation")
Set Wk7 = Wb1.Sheets("GL Mapping")
Set Wk1 = Wb2.Sheets("det")
With Wb1
With Wk5
Dim CCCol As Long, fRowCC As Long, lRowCC As Long
CCCol = Wk5.Cells.Find("Cost Center", lookat:=xlWhole).Column
fRowCC = Wk5.Cells.Find("Cost Center", lookat:=xlWhole).Offset(1, 0).Row
lRowCC = Wk5.Cells.Find("Cost Center", lookat:=xlWhole).End(xlDown).Row
Set CCRg = Wk5.Range(Wk5.Cells(fRowCC, CCCol), Wk5.Cells(lRowCC, CCCol))
Set CLinkRg = Wk5.Range(Wk5.Cells(fRowCC, CCCol).Offset(0, -1), Wk5.Cells(lRowCC, CCCol).Offset(0, -1))
End With
With Wk7
Dim MapGLCol As Long, MapfRow As Long, MaplRow As Long
MapGLCol = Wk7.Cells.Find("GL", lookat:=xlWhole).Column
MapfRow = Wk7.Cells.Find("GL", lookat:=xlWhole).Offset(1, 0).Row
MaplRow = Wk7.Cells(Rows.Count, MapGLCol).End(xlUp).Row
Dim MapGLRg As Range
Set MapGLRg = Wk7.Range(Wk7.Cells(MapfRow, MapGLCol), Wk7.Cells(MapfRow, MapGLCol))
Set TypeRg = Wk7.Range(Wk7.Cells(MapfRow, MapGLCol).Offset(0, -1), Wk7.Cells(MaplRow, MapGLCol).Offset(0, -1))
End With
With Wk4
Wk4.Outline.ShowLevels RowLevels:=2
Dim dateRow As Long, fRow2 As Long, AssumCol As Long, lRow2 As Long
dateRow = Wk4.Cells.Find("ACT", lookat:=xlWhole).Offset(1, 0).Row
fRow2 = Wk4.Cells.Find("Global Assumptions", lookat:=xlWhole).Row
AssumCol = Wk4.Cells.Find("Global Assumptions", lookat:=xlWhole).Column
lRow2 = Wk4.Cells(Rows.Count, AssumCol).End(xlUp).Row
Dim AssumptionRg As Range
Set AssumptionRg = Wk4.Range(Wk4.Cells(fRow2, AssumCol), Wk4.Cells(lRow2, AssumCol))
End With
End With
With Wb2
With Wk1
Dim OpsCol As Long
OpsCol = Wk1.Cells.Find("Property Manager", lookat:=xlWhole).Column
Dim PropCodeCol As Long
FRow = Wk1.Cells.Find("66990000", lookat:=xlPart).Offset(2, 0).Row
lRow = Wk1.Cells.Find("66990000", lookat:=xlPart).End(xlDown).Offset(-1, 0).Row
PropCodeCol = Wk1.Cells.Find("Property Code", lookat:=xlWhole).Column
Set OpsRg = Wk1.Range(Wk1.Cells(FRow, OpsCol), Wk1.Cells(lRow, OpsCol))
Dim PropCodeRg As Range
Set PropCodeRg = Wk1.Range(Wk1.Cells(FRow, PropCodeCol), Wk1.Cells(lRow, PropCodeCol))
Dim GLRow As Long, BegGLCol As Long, EndGLCol As Long
GLRow = Wk1.Cells.Find("66550000", lookat:=xlPart).Row
BegGLCol = Wk1.Cells.Find("66550000", lookat:=xlPart).Column
EndGLCol = Wk1.Cells.Find("66990000", lookat:=xlPart).Column
Dim BadDebtCol As Long
BadDebtCol = Wk1.Cells.Find("66550300", lookat:=xlPart).Column
Dim BadDebtGLRg As Range
Set BadDebtGLRg = Wk1.Range(Wk1.Cells(FRow, BadDebtCol), Wk1.Cells(lRow, BadDebtCol))
Set GLRg = Wk1.Range(Wk1.Cells(GLRow, BegGLCol), Wk1.Cells(GLRow, EndGLCol))
Dim cell As Range
For Each cell In OpsRg
If cell = "" Then
If AllEntRg Is Nothing Then
Set AllEntRg = Wk1.Cells(cell.Row, PropCodeCol)
Else
Set AllEntRg = Union(AllEntRg, Wk1.Cells(cell.Row, PropCodeCol))
End If
End If
Next cell
Set cell = Nothing
For Each cell In AllEntRg
If CCRg.Find(cell.Value, lookat:=xlWhole) Is Nothing Then
If EntityOnlyRg Is Nothing Then
Set EntityOnlyRg = Wk1.Cells(cell.Row, PropCodeCol)
Else
Set EntityOnlyRg = Union(EntityOnlyRg, Wk1.Cells(cell.Row, PropCodeCol))
End If
End If
Next cell
Set cell = Nothing
Dim EvictionRg As Range
Set EvictionRg = CategoryGLRange("Evictions ($)")
Dim CreditFeesRg As Range
Set CreditFeesRg = CategoryGLRange("Credit Verification Fees ($)")
Dim LegalCounselFeesRg As Range
Set LegalCounselFeesRg = CategoryGLRange("Legal Counsel Fees ($)")
Dim OfficeRentRg As Range
Set OfficeRentRg = CategoryGLRange("Office - Rent ($)")
Dim OfficeUtilitiesRg As Range
Set OfficeUtilitiesRg = CategoryGLRange("Office - Utilities ($)")
Dim OfficeOtherRg As Range
Set OfficeOtherRg = CategoryGLRange("Office - Other ($)")
Dim OfficeMgmtRg As Range
Set OfficeMgmtRg = Application.Union(OfficeRentRg, OfficeUtilitiesRg, OfficeOtherRg)
Dim AutoLeaseRg As Range
Set AutoLeaseRg = CategoryGLRange("Auto - Sentra/Van Fleet ($)")
Dim OtherRg As Range
Set OtherRg = CategoryGLRange("Other ($)")
End With
End With
With Wb1
With Wk4
Dim r As Range
Dim isItem As Boolean
For Each r In AssumptionRg
Select Case r
Case "Evictions ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EvictionRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, EvictionRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), EvictionRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), EvictionRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), EvictionRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), EvictionRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), EvictionRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), EvictionRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), EvictionRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), EvictionRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), EvictionRg)
End If
Case "Credit Verification Fees ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(CreditFeesRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, CreditFeesRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), CreditFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), CreditFeesRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), CreditFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), CreditFeesRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), CreditFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), CreditFeesRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), CreditFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), CreditFeesRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), CreditFeesRg)
End If
Case "Legal Counsel Fees ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(LegalCounselFeesRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, LegalCounselFeesRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), LegalCounselFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), LegalCounselFeesRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), LegalCounselFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), LegalCounselFeesRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), LegalCounselFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), LegalCounselFeesRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), LegalCounselFeesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), LegalCounselFeesRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), LegalCounselFeesRg)
End If
Case "Office - Prop Mgmt ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(OfficeMgmtRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, OfficeMgmtRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeMgmtRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), OfficeMgmtRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeMgmtRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), OfficeMgmtRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeMgmtRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), OfficeMgmtRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeMgmtRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), OfficeMgmtRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeMgmtRg)
End If
Case "Office - Rent ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(OfficeRentRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, OfficeRentRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeRentRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), OfficeRentRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeRentRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), OfficeRentRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeRentRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), OfficeRentRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeRentRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), OfficeRentRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeRentRg)
End If
Case "Office - Utilities ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(OfficeUtilitiesRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, OfficeUtilitiesRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeUtilitiesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), OfficeUtilitiesRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeUtilitiesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), OfficeUtilitiesRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeUtilitiesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), OfficeUtilitiesRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeUtilitiesRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), OfficeUtilitiesRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeUtilitiesRg)
End If
Case "Office - Other ($)"
isItem = True
If r.End(xlUp) = "Tie-Out To Actuals" Or r.End(xlUp) = "Entity Level Assumptions" _
Or r.End(xlUp) = "Inland Empire" Or r.End(xlUp) = "Atlanta East" _
Or r.End(xlUp) = "Atlanta North" Or r.End(xlUp) = "Atlanta South" Then
If r.End(xlUp) = "Tie-Out To Actuals" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(OfficeOtherRg)
End If
If r.End(xlUp) = "Entity Level Assumptions" Then
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(EntityGLRg(AllEntRg, OfficeOtherRg))
End If
If r.End(xlUp) = "Inland Empire" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeOtherRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), OfficeOtherRg))
End If
If r.End(xlUp) = "Atlanta East" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeOtherRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), OfficeOtherRg))
End If
If r.End(xlUp) = "Atlanta North" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeOtherRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), OfficeOtherRg))
End If
If r.End(xlUp) = "Atlanta South" Then
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeOtherRg) _
+ Application.WorksheetFunction.Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), OfficeOtherRg))
End If
Else
Wk4.Cells(r.Row, 4) = SinglePMRanges(r.End(xlUp), OfficeOtherRg)
End If
Case "Bad Debt ($)"
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(CategoryGLRange("Bad Debt ($)"))
End Select
Next r
Set r = Nothing
End With
End With
Application.ScreenUpdating = True
endtime = Now()
MsgBox (Format(endtime - starttime, "h:mm:ss"))
End Sub
Public Function DefMultiCCPMRange(rngSearchRange As Range, strSearchString As String) As Range
If Not AllEntRg.Find(strSearchString, lookat:=xlWhole) Is Nothing Then
Set DefMultiCCPMRange = rngSearchRange.Find(strSearchString, lookat:=xlWhole)
End If
End Function
Public Function EntityGLRg(EntityRg1 As Range, CatGLRg As Range) As Range
Dim c As Range
For Each c In EntityRg1
If CCRg.Find(c.Value, lookat:=xlWhole) Is Nothing Then
Dim c2 As Range
For Each c2 In CatGLRg
If c2.Row = c.Row Then
If EntityGLRg Is Nothing Then
Set EntityGLRg = c2
Else
Set EntityGLRg = Union(EntityGLRg, c2)
End If
End If
Next c2
End If
Next c
End Function
Public Function CategoryGLRange(TypeString As String) As Range
Dim cl As Range
For Each cl In TypeRg
If cl = TypeString Then
Dim TempCell As Range
Set TempCell = GLRg.Find(cl.Offset(0, 1).Value, lookat:=xlWhole)
If CategoryGLRange Is Nothing Then
Set CategoryGLRange = Wk1.Range(Wk1.Cells(FRow, TempCell.Column), Wk1.Cells(lRow, TempCell.Column))
Else
Set CategoryGLRange = Union(CategoryGLRange, Wk1.Range(Wk1.Cells(FRow, TempCell.Column), Wk1.Cells(lRow, TempCell.Column)))
End If
End If
Next cl
End Function
Public Function MultipleCostCenterGLRange(CostCenterRg1 As Range, CostCenterRg2 As Range, CatGLRg2 As Range) As Range
If Not CostCenterRg1 Is Nothing Or Not CostCenterRg2 Is Nothing Then
Dim cl2 As Range
For Each cl2 In CatGLRg2
If cl2.Row = CostCenterRg1.Row Or cl2.Row = CostCenterRg2.Row Then
If MultipleCostCenterGLRange Is Nothing Then
Set MultipleCostCenterGLRange = cl2
Else
Set MultipleCostCenterGLRange = Union(MultipleCostCenterGLRange, cl2)
End If
End If
Next cl2
End If
End Function
Public Function CCCodeCell(PMToCC As Range) As Range
CCCodeRow = CLinkRg.Find(PMToCC.Value, lookat:=xlWhole).Offset(0, 1).Row
CCCodeCol = CLinkRg.Find(PMToCC.Value, lookat:=xlWhole).Offset(0, 1).Column
Set CCCodeCell = Wk5.Cells(CCCodeRow, CCCodeCol)
End Function
Public Function SinglePMRanges(PMCell As Range, GLCatRg As Range) As Double
Dim Col As Range
Dim MCol As Long
Dim MRow As Long
For Each Col In GLCatRg.Columns
MCol = Col.Column
Dim TotRg As Range
Dim zz As Range
For Each zz In OpsRg
If zz = PMCell Then
If TotRg Is Nothing Then
Set TotRg = Wk1.Cells(zz.Row, MCol)
Else
Set TotRg = Union(TotRg, Wk1.Cells(zz.Row, MCol))
End If
End If
Next zz
Dim Tot2Rg As Range
If CCCodeCell(PMCell).Value <> "None" Then
If Application.WorksheetFunction.CountIf(CLinkRg, PMCell) = 1 Then
If Not DefMultiCCPMRange(AllEntRg, CCCodeCell(PMCell)) Is Nothing Then
If Tot2Rg Is Nothing Then
Set Tot2Rg = Wk1.Cells(DefMultiCCPMRange(AllEntRg, CCCodeCell(PMCell)).Row, MCol)
Else
Set Tot2Rg = Union(Tot2Rg, Wk1.Cells(DefMultiCCPMRange(AllEntRg, CCCodeCell(PMCell)).Row, MCol))
End If
End If
SinglePMRanges = Application.WorksheetFunction.Sum(TotRg, Tot2Rg)
Else
SinglePMRanges = Application.WorksheetFunction.Sum(TotRg)
End If
Else
'Do Nothing --- 3rd Party PM; No CostCenter Code to lookup and calculate GL amounts
SinglePMRanges = Application.WorksheetFunction.Sum(TotRg)
End If
Next Col
End Function
-
\$\begingroup\$ Welcome to CodeReview, fonzy16. \$\endgroup\$Legato– Legato2015年04月16日 12:51:21 +00:00Commented Apr 16, 2015 at 12:51
-
\$\begingroup\$ Please state only the code's purpose in the title. It's otherwise difficult to determine what the code is supposed to do. \$\endgroup\$Jamal– Jamal2015年04月16日 13:07:28 +00:00Commented Apr 16, 2015 at 13:07
1 Answer 1
I like to start off by saying that your formatting is pretty good. Well done. But there are a few things to simplify your code and make it easier to understand. From there it will be easier to manage and even improve the performance.
Also, the use of functions to solve smaller problems is good.
Functions
Your comments about 'public functions' are a little vague. Functions are used to improve the maintainability of code and ideally allow you to reuse the functionality in other places. It doesn't add any performance (if anything calling a function decreases performance).
So for instance a common function I use is to find the last used row in a worksheet. It serves a specific purpose, is only 5-10 lines long, but I use it all over the place when I'm coding. The cool thing is that I can replace the way it works internally without affecting any of the consumers of that function.
The use of 'Public' (and also the alternative, Private) is to control who can access a function, variable, property. In programming this is called Scope. Basically when you make something Public you are telling other code that this function is for public use.
As I mentioned above, you should separate out logical chunks of code into separate functions. Some of these functions may not be suitable for use outside of their module. In this case they can be declared Private and they won't be visible outside that module.
Use Variables for repeated code references
you have a huge number of r.End(xlUp)
references in your code. I suggest you figure out what this range represents and use a variable to name it appropriately. Similarly with Wk4.Cells(r.Row, 4)
. I have actually pulled these into a method which you will see below.
Use of With Blocks
It appears you are not understanding how With
blocks work. The reason I'm saying this is because I commented out all of the beginning and ending With
statements and the code still compiles. The purpose of these blocks is to reduce repetition of an object reference inside the containing code. So for instance instead of writing a lot of statements like this Application.WorksheetFunction.Sum( ... )
You can replace them with this:
With Application.WorksheetFunction
' Notice that we can leave out the Application.WorksheetFunction
' part and just add a leading '.'
.Sum( ... )
End With
Code Duplication
There is a huge amount of duplication in your code. Every time you use copy and paste you need to ask whether you are doing the right thing. Quite often you can see a pattern in what you are doing and can create a separate method (Function
or Sub
) that can be created to solve that particular problem. The things that are changing between uses of that code become your arguments/parameters.
Take for example the Select Case
statement inside the main loop of Maverick()
. Each case has very similar code inside of it. The only things that appear to be changing each time is the range that is used. So you could create a new method which accepts a range argument and a reference to the output workbook (WK4 in this case) and performs the processing. Somthing similar to the following:
' I have tried my best to factor out this method but I quite likely missed something,
' so carefully review this to make sure that nothing has been missed.
Private Function ProcessStuff(ByVal processRange As Range, ByVal currentRow As Range)
' Using With block to simplify calls to Sum().
With Application.WorksheetFunction
' The following two variables should be renamed to match the semantics of the data they represent.
Dim entryType As Range
entryType = currentRow.End(xlUp)
Dim outputCell As Range
Set outputCell = Wk4.Cells(currentRow.Row, 4)
' Using a select case statement instead of nested If hierarchy.
Select Case entryType
Case "Tie-Out To Actuals"
outputCell = .Sum(processRange)
Case "Entity Level Assumptions"
outputCell = .Sum(EntityGLRg(AllEntRg, processRange))
Case "Inland Empire"
outputCell = SinglePMRanges(entryType, processRange) _
+ .Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "cahied"), DefMultiCCPMRange(AllEntRg, "cahrvr"), processRange))
Case "Atlanta East"
outputCell = SinglePMRanges(entryType, processRange) _
+ .Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlse"), DefMultiCCPMRange(AllEntRg, "atle"), processRange))
Case "Atlanta North"
outputCell = SinglePMRanges(entryType, processRange) _
+ .Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlnw"), DefMultiCCPMRange(AllEntRg, "atln"), processRange))
Case "Atlanta South"
outputCell = SinglePMRanges(entryType, processRange) _
+ .Sum(MultipleCostCenterGLRange(DefMultiCCPMRange(AllEntRg, "atlsw"), DefMultiCCPMRange(AllEntRg, "atls"), processRange))
Case Else
outputCell = SinglePMRanges(entryType, processRange)
End Select
End With
End Function
Then your loop code simplifies down to:
Dim r As Range
Dim isItem As Boolean
For Each r In AssumptionRg
Select Case r
Case "Evictions ($)"
isItem = True
ProcessStuff EvictionRg, r
Case "Credit Verification Fees ($)"
isItem = True
ProcessStuff CreditFeesRg, r
Case "Legal Counsel Fees ($)"
isItem = True
ProcessStuff LegalCounselFeesRg, r
Case "Office - Prop Mgmt ($)"
isItem = True
ProcessStuff OfficeMgmtRg, r
Case "Office - Rent ($)"
isItem = True
ProcessStuff OfficeRentRg, r
Case "Office - Utilities ($)"
isItem = True
ProcessStuff OfficeUtilitiesRg, r
Case "Office - Other ($)"
isItem = True
ProcessStuff OfficeOtherRg, r
Case "Bad Debt ($)"
Wk4.Cells(r.Row, 4) = Application.WorksheetFunction.Sum(CategoryGLRange("Bad Debt ($)"))
End Select
Next r
If you end up using this code, please carefully check it.
-
\$\begingroup\$ @Rossco...Thanks for the detailed input. I just got out of morning meetings otherwise I would have responded sooner. I appreciate the insight. Everything you mentioned makes sense. I will look at your suggestion and see how I can apply it and test the results. Also, I will use try to apply that concept to consolidate to other portions of the code. Hopefully it reduces the execution time. I will let you know of the results \$\endgroup\$fonzy16– fonzy162015年04月16日 18:30:35 +00:00Commented Apr 16, 2015 at 18:30