-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Move calculation into Fingerprint model
** Why are these changes being introduced: It makes sense that the Fingerprint model is where the logic to calculate a fingerprint value is maintained, and not in the Term model. ** Relevant ticket(s): code review ** How does this address that need: This moves the fingerprint logic from the Term to Fingerprint model. Because of this, the Term model changes to use its new location (from inside the lifecycle hook where the Fingerprint record gets created). We use an instance method for this, because at the time of use there is not yet a Fingerprint record that could call the method internally. We also copy-paste the tests for this method from the SuggestedResource implementation (which should be removed in a future PR). ** Document any side effects to this change: I can think of two possible side effects: 1. the SuggestedResource implementation of the fingerprint is now very much duplicative, and should be removed (coming in a future ticket) 2. It is a bit awkward to rely on an instance method for calculating the fingerprint value. In an ideal case, register_fingerprint would use something like: self.fingerprint = Fingerprint.find_or_create_by(phrase) ... and then the Fingerprint model would: a. receive the phrase argument b. calculate the fingerprint for this phrase c. look up to see if such a record exists already, and return it d. create the record if none exists, and return _that_ A custom initialize method feels like it would work for this, but I think that's considered an antipattern in Rails? For now, I think this works.
- Loading branch information
1 parent
c05c4ed
commit 0930853
Showing
3 changed files
with
69 additions
and
21 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters