Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop splitting code across coupled modules that are only included once #497

Open
dylanahsmith opened this issue Jun 11, 2021 · 0 comments
Open
Labels

Comments

@dylanahsmith
Copy link
Contributor

dylanahsmith commented Jun 11, 2021

Currently, IdentityCache::WithoutPrimaryIndex has the following single use internal module includes

    include IdentityCache::BelongsToCaching
    include IdentityCache::CacheKeyGeneration
    include IdentityCache::ConfigurationDSL
    include IdentityCache::QueryAPI
    include IdentityCache::CacheInvalidation
    include IdentityCache::ShouldUseCache
    include ParentModelExpiration

This is an anti-pattern that comes from not having proper separation of code into separate classes. To make things even worse, all this code is being included into model classes, so making any of the methods private doesn't actually prevent application code from calling them.

A better pattern to follow is the one taken by the classes in the IdentityCache::Cached namespace. They are built during initialization, which avoids adding extra object allocation on hot code paths, but still provides a place to store configuration state and methods that use that state. There may be more methods we can move into those classes and there may be opportunities to create new classes that are decoupled from the model classes.

The remaining methods on those single use internal module should be moved into IdentityCache::WithoutPrimaryIndex.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant