Closed ZuZuK closed 4 years ago
Thanks for the PR!
I'm not sure how I feel about this though. Not only does it not have any tests, but its diff is quite involved, and it adds a lot of functionality that the library already covers, kind of. There is an EnumValueTypeConverter
available through LoganSquare directly, and if its toString()
implementation doesn't cut it, usually you can get by with a small Mapper function & another TypeConverter
.
For example:
public enum MyEnum {
FIRST(1),
SECOND(2),
OTHER(0);
private final int val;
MyEnum(int val) {
this.val = val;
}
public int getValue() {
return val;
}
public static MyEnum from(int val) {
for (MyEnum enum : values()) {
if (enum.val == val) {
return enum;
}
}
return OTHER;
}
}
public class MyEnumTypeConverter extends IntBasedTypeConverter<MyEnum> {
@Override
public MyEnum getFromInt(int value) {
return MyEnum.from(value);
}
@Override
public int convertToInt(MyEnum object) {
return object.getValue();
}
}
All of the added enum value annotations serve a niche that the library already has a decent way of supporting, in my opinion.
Just my two cents. What do you think, @EricKuck?
Thanks for comment, @aurae Tests is not a problem - I'll write it soon, just forgot about that :(
So you need to do 3 things 1) create enum and add some code in it 2) create converter 3) register converter
and I did such PR because of such code in every project: http://puu.sh/vuFNy/67c926a64a.png plus for every enum http://puu.sh/vuFOT/1c412e7ca3.png
instead of that I could just add one annotation per enum + one annotation per value of each enum. basically I can even not annotate values if their names equals values in json
Unit tests added
Thanks, the tests are looking good!
I have to agree that the manual type registration of enums is a little tiresome. We've got about 10 of these in our app as well! I'm still not certain about the plethora of annotations added for only this use-case, though - especially for the value annotations. Perhaps we can draft a simpler interface, maybe re-use @JsonObject
for the class, and use something like @JsonEnum(Object)
as a unified gateway for value annotations?
@aurae
that's not a problem to replace @JsonEnum
->@JsonObject
.
but @JsonObject
annotation have fieldDetectionPolicy
, fieldNamingPolicy
, serializeNullObjects
and serializeNullCollectionElements
parameters and not sure that it will be clear to developers that these parameters are not for enums but for objects.
But it is possible to shout warning on annotation processing stage like fieldDetectionPolicy is ignoring because @JsonObject is annotating enum
or something like that.
And for values - I would be happy if I could pass Object
as parameter but Java not allows this - allows only primitive/String/Class/Enum/Annotation/array.
Can only do different annotations for String/boolean/long.
Maybe it's better to rename annotations to something like JsonValue
for strings (as it is common case), JsonBooleanValue
, JsonNumberValue
. I'm not good in naming :(
With that PR it is possible to let processor generate code for enum parsing.
It will create type converter (same way like mappers) named with suffix $$JsonTypeConverter. For enums parser will firstly check if such converter added in
LoganSquare.ENUM_CONVERTERS
map then if classClass.forName(enumClass.getName() + Constants.CONVERTER_CLASS_SUFFIX)
exists. Similar to mappers logic.To create enum model yo need to write something like
No specific code to use such enum model:
By default it is associating name of enum value with string value coming from JSON. But it is possible to specify ValueNamingPolicy in
@JsonEnum
annotation.Also it is possible to specify concrete string/boolean/number associated with enum value:
More than that it is also possible to specify null associated with enum value - it means, if null have came from server then it will be specific enum value in field on parsing. And on serialization if it will be such value in field so client will send null to server.
Not sure that code is acceptable, feature design is good etc. Need comments)