Cascading Changes to Future Entries in a Schedule

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 inner

Arrow AntiPattern

Yes 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 OnCascadeChanges

End Sub
Private 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 Sub
Private Function IsOutDated(ByVal entry As SmartScheduleEntry) As Boolean
    IsOutDated = (entry.WeekDay = mOldWeekDay And _
                  entry.Week = mOldWeek And _
                  entry.Team = mOldTeam)
End Function

You 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 Function

Public Sub DoCascade(ByRef entryFrom As SmartScheduleEntry, _
        ByRef entryTo As SmartScheduleEntry

    With entryTo

            .Team = entryFrom.Team
            .Week = entryFrom.Week
            .WeekDay = entryFrom.WeekDay

    End With

End Sub

IsDirty is Dirty

Looking 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 Property

The 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.

StackExchange Code Review Q#72033, answer score: 3