Cache the value of changed_attributes when calling changes_applied

[sgrif]

changes_applied calles changes, which will call changed_attributes
multiple times in a loop. This method actually performs work now, so we
should cache the results while looping over it when we know it cannot
change.

[jeremy]

Maybe #changes should accept a hash of changed attrs, instead, that defaults to changed_attributes? The construction here is admirable but surprising & brittle, too.

[sgrif]

@jeremy That would also require passing it to every method that changes calls, which is nearly every method in dirty checking. I'm not opposed to passing it in optionally, but it's fairly sweeping.

[egilburg]

I'd rename it to caching_changed_attributes because cache_ implies it will be persisted and not just set for duration of method execution.

[chancancode]

with_cached_changed_attributes then? 😄

[jeremy]

@sgrif How sweeping? Does it indicate an awkwardness in our implementation, in our state management, or ?

Needing to set temporary instance state that implicitly affects other methods gets the job done, but by the same token it indicates that our design relies on implicitly consistent instance state rather than "snapshotting" the current changes and acting on them.

[sgrif]

Does it indicate an awkwardness in our implementation, in our state management, or ?

Yes, and I'm planning on fixing that, but the refactoring I have in mind has more pre-requisites that I'm working on atm...

Needing to set temporary instance state that implicitly affects other methods gets the job done, but by the same token it indicates that our design relies on implicitly consistent instance state rather than "snapshotting" the current changes and acting on them.

I agree, not happy with the design long-term. I'm actually not happy with the in-place implementation ATM, but I can't write the code I want to yet. Working on it.

[sgrif]

@jeremy Do I have additional action items that I need to do on this?

[jeremy]

@sgrif I was kinda hoping someone would pipe up with a better way 😁 Doesn't feel good to introduce a workaround like this and know it'll stick with us indefinitely.

[sgrif]

and know it'll stick with us indefinitely

Except I've been actively working on this stuff for months, and said its on my radar?

[jeremy]

@sgrif True that! ❤️