fury icon indicating copy to clipboard operation
fury copied to clipboard

[Java] FuryBuilder supports registered classes

Open LiangliangSui opened this issue 1 year ago • 8 comments

Is your feature request related to a problem? Please describe.

Currently, classes can only be registered through the Fury#register interface. Could we build Fury and register the class in one go?

Some users have reported that classes cannot be registered through FuryBuilder. #1458

Describe the solution you'd like

Could we support chained(In the process of building Fury) class registration in FuryBuilder?

// e.g. 
Fury fury = Fury.builder().withLanguage(Language.JAVA)
        .requireClassRegistration(true)
        // --------- Chain execution registration.
        .register(Foo.class)
        .register(Foo1.class)
        .build();

Additional context

N / A

LiangliangSui avatar Apr 04 '24 02:04 LiangliangSui

Looks good to me, we can add API for register class by order, register class by id, and register serializer by class.

chaokunyang avatar Apr 04 '24 14:04 chaokunyang

One concern is that BaseFury already supports register classes/serializers, do we still need to add this API to FuryBuilder?

chaokunyang avatar Apr 04 '24 15:04 chaokunyang

One concern is that BaseFury already supports register classes/serializers, do we still need to add this API to FuryBuilder?

We can extract all register(...) methods in BaseFury into an interface, for example named ClassRegister, and then let BaseFury extends ClassRegister, and FuryBuilder can also implements ClassRegister. registerSerializer(...) is similar, we can extract an interface named SerializerRegister.

WDYT? @chaokunyang

LiangliangSui avatar Apr 06 '24 15:04 LiangliangSui

I perfer not. Fury is not a ClassRegister, there is no such a straightforward is-a relationship between these two classes. And it's not that intuitive for a builder class to implement some interface.

chaokunyang avatar Apr 07 '24 02:04 chaokunyang

I agree with you, Fury and ClassRegister have a has-a relationship.

If we don't implement some interfaces in FuryBuilder, then we can only repeatedly define the registerXXX function, or do you have some other good suggestions?

LiangliangSui avatar Apr 07 '24 03:04 LiangliangSui

I still don't know why users can't invoke BaseFury to register a class/serializer? If we need a new inferface for this registration. It looks a little too heavy to me. The basic principle is try to avoid unnecessary abstraction

chaokunyang avatar Apr 07 '24 12:04 chaokunyang

It is also possible to use BaseFury to register classes and Serializers, but these steps need to be done after Fury build. It is best to register during the FuryBuilder build process, but currently, this will cause the overall structure to be very heavy (in order to achieve this function will lead to architectural redundancy).

Currently, only one user has made this suggestion. We can also observe whether other users make this suggestion in the future. If so, we will consider adjusting the structure to implement this function.

We can keep this issue for continuous observation to see if there are any similar suggestions.

LiangliangSui avatar Apr 07 '24 13:04 LiangliangSui

Ok, we can keep the issue open. If the demand for registering classes on FuryBuilder continues grow, we can support it later. And all similar methods such as setClassChecker/setSerializerFactory have similar issues. We should take those API into consideration too.

I believe we will receive more feedbacks when we made several releases under ASF.

chaokunyang avatar Apr 07 '24 13:04 chaokunyang