Shopify / Shopify/identity_cache
Stop splitting code across coupled modules that are only included once
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 2k
- Forks
- 174
- Avg merge
- 13m
- Merged PRs (30d)
- 3
Description
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with IdentityCache::WithoutPrimaryIndex and review the listed included modules, then compare their responsibilities with the classes in the IdentityCache::Cached namespace. Done means the remaining methods from the single-use internal modules have been moved into IdentityCache::WithoutPrimaryIndex, with any additional class boundaries identified and validated by the project’s existing tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100