I've been working on a scheduling application and I have the middle tier completed at this point. It's not changed in a few days, so I feel it's ready for review. I have just this one routine that feels dirty. It's definitely verging on arrow code, but without short-circuiting, I'm not sure how it can be improved any farther.My Schedule class wraps a collection of ScheduleEntries and provides methods to add entries, remove entries, and cascade changes (as well as a way to listen for changes to the underlying collection). When CascadeChanges is called, the collection of entries is searched for dirty records. Those records are then cascaded to  the corresponding records in future Cycles. A number of conditions must be met in order to ensure changes are being cascaded to the correct future entries. Currently, I have sacrificed an amount of performance for cleaner, more readable code. How can this method be improved?Public Sub CascadeChanges()    Dim innerEntries As SmartScheduleEntries    Set innerEntries = Me.Entries    '??? use group id to cascade changes?    Dim entry As SmartScheduleEntry    For Each entry In Me.Entries        If entry.IsDirty Then            Dim innerEntry As SmartScheduleEntry            For Each innerEntry In innerEntries                If innerEntry.Store = entry.Store Then                    If (innerEntry.Cycle.Year = entry.Cycle.Year _                        And innerEntry.Cycle.Number > entry.Cycle.Number) _                    Or innerEntry.Cycle.Year > entry.Cycle.Year Then                        With innerEntry                            If .WeekDay = mOldWeekDay And .Week = mOldWeek And .Team = mOldTeam Then                                .Team = entry.Team                                .Week = entry.Week                                .WeekDay = entry.WeekDay                            End If                        End With                    End If                End If            Next innerEntry        End If    Next entry    RaiseEvent OnCascadeChangesEnd SubHere are my two test cases. (I've been using Rubberduck to unit test all of this.)'@TestMethodPublic Sub CascadeShouldUpdateFuture()    On Error GoTo TestFailArrange:    Dim mock As SmartSchedule    Set mock = Mocks.MockFullSchedule    Dim originalDay As VbDayOfWeek    originalDay = mock.Entries(1).WeekDay    Dim shouldBeChanged As New SmartScheduleEntries    Dim entry As SmartScheduleEntry    For Each entry In mock.Entries        If entry.WeekDay = originalDay And entry.Store = 6003 Then            shouldBeChanged.Add entry, entry.ID        End If    NextAct:    mock.Entries(1).WeekDay = vbFriday ' make a change to first record    mock.CascadeChangesAssert:    For Each entry In shouldBeChanged        Assert.AreEqual vbFriday, entry.WeekDay, Cycle:  & entry.Cycle.ToString    NextTestExit:    Exit SubTestFail:    If Err.Number <> 0 Then        Assert.Fail Test raised an error: # & Err.Number &  -  & Err.Description    Else        Resume TestExit    End IfEnd Sub'@TestMethodPublic Sub CascadeShouldNotUpdatePast()    On Error GoTo TestFailArrange:    Dim mock As SmartSchedule    Set mock = Mocks.MockFullSchedule    Dim originalDay As VbDayOfWeek    originalDay = mock.Entries(1).WeekDay    Dim shouldNotBeChanged As New SmartScheduleEntries    Dim entry As SmartScheduleEntry    For Each entry In mock.Entries        If entry.WeekDay <> originalDay And entry.Store <> 6003 Then            shouldNotBeChanged.Add entry, entry.ID        End If    NextAct:    mock.Entries(1).WeekDay = vbFriday ' make a change to first record    mock.CascadeChangesAssert:    For Each entry In shouldNotBeChanged        Assert.AreNotEqual vbFriday, entry.WeekDay, Cycle:  & entry.Cycle.ToString & ; Store:  & entry.ToString    NextTestExit:    Exit SubTestFail:    If Err.Number <> 0 Then        Assert.Fail Test raised an error: # & Err.Number &  -  & Err.Description    Else        Resume TestExit    End IfEnd SubFor context, below you will find the relevant classes. I'm happy to receive criticism on these, but I'm pretty happy with them as they are.Schedule:Option ExplicitPrivate WithEvents mEntries As SmartScheduleEntriesPublic Event OnAddEntry(ByRef entry As SmartScheduleEntry)Public Event OnRemoveEntry(ByRef entry As SmartScheduleEntry)Public Event OnCascadeChanges()Private mOldWeek As CycleWeekPrivate mOldWeekDay As VbDayOfWeekPrivate mOldTeam As StringPublic Property Get Entries() As SmartScheduleEntries    Set Entries = mEntriesEnd PropertyPublic Property Set Entries(ByVal value As SmartScheduleEntries)    Set mEntries = valueEnd PropertyPublic Sub AddEntry(ByVal entry As SmartScheduleEntry)    mEntries.Add entry, entry.ID    RaiseEvent OnAddEntry(entry)End SubPublic Sub RemoveEntry(ByVal entry As SmartScheduleEntry)    mEntries.Remove entry    RaiseEvent OnRemoveEntry(entry)End SubPublic Sub Validate()    'todo: implement Validate()    RaiseNotImplementedError ValidateEnd SubPublic Sub CascadeChanges()    Dim innerEntries As SmartScheduleEntries    Set innerEntries = Me.Entries    '??? use group id to cascade changes?    Dim entry As SmartScheduleEntry    For Each entry In Me.Entries        If entry.IsDirty Then            Dim innerEntry As SmartScheduleEntry            For Each innerEntry In innerEntries                If innerEntry.Store = entry.Store Then                    If (innerEntry.Cycle.Year = entry.Cycle.Year _                        And innerEntry.Cycle.Number > entry.Cycle.Number) _                    Or innerEntry.Cycle.Year > entry.Cycle.Year Then                        With innerEntry                            If .WeekDay = mOldWeekDay And .Week = mOldWeek And .Team = mOldTeam Then                                .Team = entry.Team                                .Week = entry.Week                                .WeekDay = entry.WeekDay                            End If                        End With                    End If                End If            Next innerEntry        End If    Next entry    RaiseEvent OnCascadeChangesEnd SubPublic Sub CleanEntries()    Dim entry As SmartScheduleEntry    For Each entry In mEntries        entry.IsDirty = False    NextEnd SubPrivate Sub Class_Initialize()    Set mEntries = New SmartScheduleEntriesEnd SubPrivate Sub mEntries_Add(ByRef entry As SmartScheduleEntry)    ' ReRaises event    RaiseEvent OnAddEntry(entry)End SubPrivate Sub mEntries_ItemChanged(ByRef outWeek As CycleWeek, ByRef outWeekDay As VbDayOfWeek, ByRef outTeam As String)    mOldWeekDay = outWeekDay    mOldWeek = outWeek    mOldTeam = outTeamEnd SubPrivate Sub mEntries_Remove(ByRef entry As SmartScheduleEntry)    ' ReRaises Event    RaiseEvent OnRemoveEntry(entry)End SubPrivate Sub RaiseNotImplementedError(ByVal procName As String)    Err.Raise vbObjectError + 1, TypeName(Me) & . & procName, Not implemented yet.End SubEntry:Option ExplicitPublic Enum ScheduleEntryError    ReadOnlyPropertyError = vbObjectError + 3333End EnumPublic Enum CycleWeek    weekOne = 1    WeekTwoEnd EnumPrivate Type TScheduleEntry    ID As Long    GroupID As Long    Cycle As Cycle    Team As String    Store As Integer    WeekDay As VbDayOfWeek    Week As CycleWeek    IsDirty As BooleanEnd TypePrivate this As TScheduleEntryPublic Event OnWeekDayChange(ByRef outDay As VbDayOfWeek)Public Event OnWeekChange(ByRef outWeek As CycleWeek)Public Event OnTeamChange(ByRef outTeam As String)Public Property Get ID() As Long    ID = this.IDEnd PropertyPublic Property Let ID(ByVal value As Long)    If this.ID = 0 Then        this.ID = value    Else        RaiseReadOnlyError ID    End IfEnd PropertyPublic Property Get GroupID() As Long    GroupID = this.GroupIDEnd PropertyPublic Property Let GroupID(ByVal value As Long)    If this.GroupID = 0 Then        this.GroupID = value    Else        RaiseReadOnlyError GroupID    End IfEnd PropertyPublic Property Get IsDirty() As Boolean    IsDirty = this.IsDirtyEnd PropertyPublic Property Let IsDirty(ByVal value As Boolean)    this.IsDirty = valueEnd PropertyPublic Property Get Team() As String    Team = this.TeamEnd PropertyPublic Property Let Team(ByVal value As String)    Dim old As String    old = this.Team    this.Team = value    this.IsDirty = True    RaiseEvent OnTeamChange(old)End PropertyPublic Property Get Store() As Integer    Store = this.StoreEnd PropertyPublic Property Let Store(ByVal value As Integer)    this.Store = value    this.IsDirty = TrueEnd PropertyPublic Property Get Cycle() As Cycle    Set Cycle = this.CycleEnd PropertyPublic Property Set Cycle(ByVal value As Cycle)    Set this.Cycle = value    this.IsDirty = TrueEnd PropertyPublic Property Get Week() As CycleWeek    Week = this.WeekEnd PropertyPublic Property Let Week(ByVal value As CycleWeek)    Dim old As CycleWeek    old = this.Week    this.Week = value    this.IsDirty = True    RaiseEvent OnWeekChange(old)End PropertyPublic Property Get WeekDay() As VbDayOfWeek    WeekDay = this.WeekDayEnd PropertyPublic Property Let WeekDay(ByVal value As VbDayOfWeek)    Dim old As VbDayOfWeek    old = this.WeekDay    this.WeekDay = value    this.IsDirty = True    RaiseEvent OnWeekDayChange(old)End Property'read-only propertyPublic Property Get SetDate() As Date    Dim result As Date    ' vbMonday == 2, and our week starts on Monday.    '   If DayOfWeek == vbMonday, it is the startdate, we should add zero days.    '   In other words, Add (2 - 2) to startdate if it's Monday.    If this.Week = weekOne Then        result = DateAdd(d, this.WeekDay - 2, this.Cycle.StartDate)    Else        result = DateAdd(d, this.WeekDay - 2 + 7, this.Cycle.StartDate)    End If    SetDate = resultEnd PropertyPublic Function ToString() As String    ToString = this.Cycle.ToString & , & this.Team & , & this.Store & , & this.Week & , & this.WeekDay & , & this.IsDirtyEnd FunctionPrivate Sub RaiseReadOnlyError(ByVal procName As String)    Err.Raise ScheduleEntryError.ReadOnlyPropertyError, TypeName(Me) & . & procName, Property Is ReadOnly.End SubEntries Collection:VERSION 1.0 CLASSBEGIN  MultiUse = -1  'TrueENDAttribute VB_Name = SmartScheduleEntriesAttribute VB_GlobalNameSpace = FalseAttribute VB_Creatable = FalseAttribute VB_PredeclaredId = FalseAttribute VB_Exposed = FalseOption ExplicitPrivate mCollection As CollectionPrivate WithEvents mEntryListener As SmartScheduleEntryAttribute mEntryListener.VB_VarHelpID = -1Public Event Added(ByRef entry As SmartScheduleEntry)Public Event Removed(ByRef entry As SmartScheduleEntry)Public Event ItemChanged(ByRef outWeek As CycleWeek, ByRef outWeekDay As VbDayOfWeek, ByRef outTeam As String)Public Function Add(ByRef entry As SmartScheduleEntry, ByVal Key As Long)    mCollection.Add entry, CStr(Key)    RaiseEvent Added(entry)End FunctionPublic Function Remove(ByVal entry As SmartScheduleEntry)    mCollection.Remove IndexOf(entry)    RaiseEvent Removed(entry)End FunctionPublic Function Item(ByVal index As Variant) As SmartScheduleEntryAttribute Item.VB_UserMemId = 0    Set mEntryListener = mCollection(index)    Set Item = mEntryListenerEnd FunctionPublic Function Count() As Long    Count = mCollection.CountEnd Function' returns index of item if found, returns 0 if not foundPublic Function IndexOf(ByVal entry As SmartScheduleEntry) As Long    Dim i As Long    For i = 1 To mCollection.Count        If mCollection(i).ID = entry.ID Then            IndexOf = i            Exit Function        End If    NextEnd FunctionPublic Function NewEnum() As IUnknownAttribute NewEnum.VB_UserMemId = -4    Set NewEnum = mCollection.[_NewEnum]End FunctionPrivate Sub Class_Initialize()    Set mCollection = New CollectionEnd SubPrivate Sub Class_Terminate()    Set mCollection = NothingEnd SubPrivate Sub mEntryListener_OnTeamChange(ByRef outTeam As String)    RaiseEvent ItemChanged(mEntryListener.Week, mEntryListener.WeekDay, outTeam)End SubPrivate Sub mEntryListener_OnWeekChange(ByRef outWeek As CycleWeek)    RaiseEvent ItemChanged(outWeek, mEntryListener.WeekDay, mEntryListener.Team)End SubPrivate Sub mEntryListener_OnWeekDayChange(ByRef outDay As VbDayOfWeek)    RaiseEvent ItemChanged(mEntryListener.Week, outDay, mEntryListener.Team)End SubCycle:Option ExplicitPrivate Type TCycle    StartDate As Date    EndDate As Date    Year As Integer    Number As IntegerEnd TypePrivate this As TCyclePublic Property Get Year() As Integer    Year = this.YearEnd PropertyPublic Property Let Year(ByVal value As Integer)    this.Year = valueEnd PropertyPublic Property Get Number() As Integer    Number = this.NumberEnd PropertyPublic Property Let Number(ByVal value As Integer)    this.Number = valueEnd PropertyPublic Property Get StartDate() As Date    StartDate = DateValue(this.StartDate)End PropertyPublic Property Let StartDate(ByVal value As Date)    this.StartDate = valueEnd PropertyPublic Property Get EndDate() As Date    EndDate = DateValue(this.EndDate)End PropertyPublic Property Let EndDate(ByVal value As Date)    this.EndDate = valueEnd PropertyPublic Function ToString() As String    ToString = this.Year & -P & Format(this.Number, 00)End FunctionPublic Sub SetFromString(ByVal value As String)    Dim arr As Variant    arr = Split(value, -P, 2)    Me.Year = arr(0)    Me.Number = arr(1)End Sub
Cascading Changes to Future Entries in a Schedule
vba
Arrow AntiPatternYes your arrow code is dirty it can be broken down into other methods.  They maybe only used in one method now but as your code expands you will find it convenient that these methods are already defined.  I find keeping every method to one or two control structures helps.  Please better names should be used than what I used as I don't fully understand your product.Public Sub CascadeChanges()    Dim entries As SmartScheduleEntries    Set entries = Me.Entries    Dim entry As SmartScheduleEntry    For Each entry in entries         If entry.IsDirty Then CascadeEntry entry, entries    Next entry    RaiseEvent OnCascadeChangesEnd SubPrivate Sub CascadeEntry(ByVal inputEntry As SmartScheduleEntry, _        ByVal entries As SmartScheduleEntries)    Dim entry As SmartScheduleEntry    For Each entry In entries        If OughtCascade(inputEntry, entry) And IsOutDated(entry) Then             DoCascade inputEntry, entry        End If    Next entry End SubPrivate Function IsOutDated(ByVal entry As SmartScheduleEntry) As Boolean    IsOutDated = (entry.WeekDay = mOldWeekDay And _                  entry.Week = mOldWeek And _                  entry.Team = mOldTeam)End FunctionYou might want to abstract various comparisons of OughtCascade out but I do know which are relevant enough to abstract.  All of the comparisons are just simple properties, so the lack of short circuit evaluation has marginal cost. Looking back into your Scheduler class, not all of these methods belong in that class.  The following two could be ported to your SmartScheduleEntry class.Private Function OughtCascade(ByVal entryFrom SmartScheduleEntry, _        ByVal entryTo SmartScheduleEntry) As Boolean    OughtCascade = (entryFrom.Store = entryTo.Store) And _                   ((entryFrom.Cycle.Year = entryTo.Cycle.Year) And _                   (entryFrom.Cycle.Number < entryTo.Cycle.Number) Or _                   (entryFrom.Cycle.Year < entryTo.Cycle.Year))End FunctionPublic Sub DoCascade(ByRef entryFrom As SmartScheduleEntry, _        ByRef entryTo As SmartScheduleEntry    With entryTo            .Team = entryFrom.Team            .Week = entryFrom.Week            .WeekDay = entryFrom.WeekDay    End WithEnd SubIsDirty is DirtyLooking further at your code I am becoming suspicious of the IsDirty member.  I believe the property should evaluate whether the entry is dirty or not and not read from a member.  It appears to be causing boiler plate code in the Let properties of other members.Public Property Let WeekDay(ByVal value As VbDayOfWeek)    Dim old As VbDayOfWeek    old = this.WeekDay    this.WeekDay = value    this.IsDirty = True    RaiseEvent OnWeekDayChange(old)End PropertyThe issue is Get IsDirty is dependent on the code of Let WeekDay and other properties.  Get IsDirty should be independent on any methods it does not specifically reference.  Isolating IsDirty may require completely redesigning your structure.  Seeing as IsDirty seems to be synonymous with HasMutated, consider making your SmartScheduleEntry class immutable.