3
\$\begingroup\$

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
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Apr 16, 2015 at 7:44
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to CodeReview, fonzy16. \$\endgroup\$ Commented 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\$ Commented Apr 16, 2015 at 13:07

1 Answer 1

5
\$\begingroup\$

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.

answered Apr 16, 2015 at 11:27
\$\endgroup\$
1
  • \$\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\$ Commented Apr 16, 2015 at 18:30

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.