-
Notifications
You must be signed in to change notification settings - Fork 2
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
DRAFT - Support comparable entity fields and comparison DSL #88
base: master
Are you sure you want to change the base?
Conversation
main/src/main/java/com/kenshoo/pl/entity/AbstractEntityType.java
Outdated
Show resolved
Hide resolved
main/src/main/java/com/kenshoo/pl/entity/AbstractEntityType.java
Outdated
Show resolved
Hide resolved
main/src/main/java/com/kenshoo/pl/entity/ComparableEntityField.java
Outdated
Show resolved
Hide resolved
3cfb38d
to
7c3f605
Compare
7c3f605
to
695258c
Compare
E extends EntityType<E>, | ||
T, | ||
F extends EntityField<E, T>, | ||
B extends AbstractEntityField.Builder<E, T, F, B>> implements MutableEntityField<E, T> { |
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.
To make the builder inheritance work smoothly without casting, I needed to do the following:
- Add extra generic params for the field and builder types
- Add this abstract base class on top of all existing ones (it doesn't work well when the base class is the concrete
EntityFieldImpl
)
@@ -14,6 +21,9 @@ public class TestEntity extends AbstractEntityType<TestEntity> { | |||
public static final PrototypedEntityField<TestEntity, Integer> FIELD_3 = INSTANCE.prototypedField(TestDataFieldPrototype.FIELD_3, TestEntityTable.TABLE.field_3); | |||
public static final EntityField<TestEntity, String> SECONDARY_FIELD_1 = INSTANCE.field(SecondaryTable.TABLE.secondary_field_1); | |||
|
|||
public static final EntityField<TestEntity, BigDecimal> CUSTOM_COMPARABLE_FIELD = | |||
INSTANCE.field(TestEntityTable.TABLE.field_4).comparedBy(comparing(n -> n.setScale(2, DOWN))); |
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.
A dummy example of how to configure a custom comparator - not used in any tests
} | ||
throw new UnsupportedOperationException(String.format("The field [%s] is not comparable for the value [%s]", this, value)); | ||
}); | ||
} |
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 implementation is designed to add support for comparison operations for all fields as much as possible.
- If the field is comparable:
- It will use the natural comparator by default.
- If the user overrode this, it will use the specific one
- If the field is not comparable - an exception will be thrown
public interface MutableEntityField<E extends EntityType<E>, T> extends EntityField<E, T> { | ||
|
||
EntityField<E, T> comparedBy(final Comparator<T> valueComparator); | ||
} |
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 interface is needed for the DSL to work properly - after a field has been created with field()
we need to be able to optionally add a comparator. On the other hand we don't want this to be part of the regular EntityField
interface used in many places that should be effectively 'immutable'
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.
@effiban
Let's call this interface ComparableEntityField
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.
The problem is that it is only comparable after you call the method. If you use this name, then every field we already have today will implement this interface, which might be confusing since some of them really are not comparable.
Need to think of something similar that means "optionally comparable"
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass") | ||
public boolean equals(Object o) { | ||
return (this == o); | ||
} |
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.
Removed this because it is the default equals implementation (if I'm not mistaken), and I didn't put it in the new base class either
|
||
@Override | ||
public int hashCode() { | ||
return System.identityHashCode(this); |
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.
as above, I don't know why this was overriden
No description provided.