Add truncation filter strategy for all filters #180
Conversation
JessieAMorris
force-pushed
the
feature/truncate-mask
branch
from
5cd9a91 to
4492011
Compare
JessieAMorris
force-pushed
the
feature/truncate-mask
branch
from
4492011 to
2e32224
Compare
| @@ -107,6 +110,18 @@ public abstract class AbstractFilterStrategy { | |||
| @Expose | |||
| protected String maskLength = SAME; | |||
|
|
|||
| @SerializedName("truncateDigits") | |||
| @Expose | |||
| protected Integer truncateDigits; | |||
There was a problem hiding this comment.
Do you think truncateLength is more descriptive? I noticed on line 393 you did refer to it as length in the error message.
When I first read it I interpreted it as "the specific digits you want to truncate" even though that doesn't make a lot of sense.
There was a problem hiding this comment.
I was mainly matching what the zip filter strategy already had, but I'm not sure I love either digits or length. I would have expected based on the name to have it truncate N digits rather than leaving N digits.
Maybe something like truncateLeaveCharacters or something would be more intuitive?
There was a problem hiding this comment.
Oh, yes, truncateDigits is what's in the zip code filter. I forgot about that. That used to make sense and now I don't like it.
I see your point. truncateLeaveCharacters makes sense. I think that's clearer. I'll write a new issue to make the property in ZipCodeFilterStrategy consistent with what you use here.
There was a problem hiding this comment.
I changed the parameter name to truncateLeaveCharacters in both the new code as well as the ZipCodeFilterStrategy. I made it backwards compatible so if truncateDigits is specified it will be used, but marked it as deprecated. I updated the docs to reference truncateLeaveCharacters.
|
Looks great and definitely a valid use-case! Just one question about the property name. |
Description
I have a use case where I want to filter out specific strings only leveraging the identifier filter. However, I'd like a
LAST_4style filter strategy for the identifier filter. Thetruncatefilter more or less did what I wanted, but it was previously only supported for thezipfilter. It also only supported leaving leading characters.This change adds support for
TRUNCATEon the other filters and adds atruncateDirectionfor specifying if leading or trailing characters should be left.Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.