LiangliangSui commented on code in PR #1406:
URL: https://github.com/apache/incubator-fury/pull/1406#discussion_r1527106514


##########
java/fury-core/src/main/java/org/apache/fury/resolver/ClassResolver.java:
##########
@@ -391,12 +391,7 @@ private void registerDefaultClasses() {
 
   /** register class. */
   public void register(Class<?> cls) {
-    if (!extRegistry.registeredClassIdMap.containsKey(cls)) {
-      while 
(extRegistry.registeredId2Classes.containsKey(extRegistry.registeredClassIdCounter))
 {
-        extRegistry.registeredClassIdCounter++;
-      }
-      register(cls, extRegistry.registeredClassIdCounter);
-    }
+    register(cls, extRegistry.registeredClassIdCounter);

Review Comment:
   > Yes, registered class id should be exposed to users, so they can know 
which ids they could use to avoid the conflict
   
   We can abstract a separate class for ClassID allocation, provide an 
interface for users to obtain ClassID, and then reference this class in 
`ClassResolver` (Currently, the ClassResolver class is responsible for a lot of 
content, and this can also reduce the burden on the `ClassResolver` class).
   
   In this way, we have one more class(For example named `ClassIdAllocator`) 
responsible for maintaining ClassID allocation, which can be used internally 
(`ClassResolver`) or externally (`Fury#allocateClassId`).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to