-
Notifications
You must be signed in to change notification settings - Fork 80
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
RYA-165 Simplify PCJ result visibility expressions before exporting t… #90
Conversation
* @param visibility - The expression to simplify. (not null) | ||
* @return A simplified form of {@code visibility}. | ||
*/ | ||
public String simplify(final String visibility) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I feel like this is how it should've been implemented in flatten()
everything looks good to me. I like the changes. We using Objects.requireNonNull() inplace of Preconditions moving forward? I still see both used in various places |
@@ -54,6 +58,16 @@ | |||
private static final VisibilityBindingSetStringConverter CONVERTER = new VisibilityBindingSetStringConverter(); | |||
|
|||
/** | |||
* Simplifies Visibility expressions prior to exporting PCJ results. | |||
*/ | |||
private static final VisibilitySimplifier SIMPLIFIER = new VisibilitySimplifier(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason the simplify method isn't static (why are we creating this class)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static code is harder to test and extend, so I just shy away from it.
@@ -32,28 +30,36 @@ | |||
public class VisibilityBindingSetStringConverter extends BindingSetStringConverter { | |||
public static final char VISIBILITY_DELIM = 1; | |||
|
|||
private static final int BINDING_SET_STRING_INDEX = 0; | |||
private static final int VISIBILITY_EQUATION_INDEX = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an expression, not an equation.
…hem to the Rya PCJ index table.
looks good. about to merge. |
@amihalik
@isper3at