fishy commented on a change in pull request #2439:
URL: https://github.com/apache/thrift/pull/2439#discussion_r725557753



##########
File path: lib/java/src/org/apache/thrift/TDeserializer.java
##########
@@ -61,6 +78,54 @@ public TDeserializer(TProtocolFactory protocolFactory) 
throws TTransportExceptio
     protocol_ = protocolFactory.getProtocol(trans_);
   }
 
+  /**
+   * Construct a new TDeserializer that supports partial deserialization
+   * that outputs instances of type controlled by the given {@code processor}.
+   *
+   * @param thriftClass a TBase derived class.
+   * @param fieldNames list of fields to deserialize.
+   * @param processor the Processor that handles deserialized field values.
+   * @param protocolFactory the Factory to create a protocol.
+   */
+  public TDeserializer(

Review comment:
       I disagree. Imagine this PR is merged and some developer wrote this code:
   
   ```java
   TDeserializer deserializer = new TDeserializer(thriftClass, fieldNames, 
processor, protocolFactory);
   deserializer.deserialize(base, buf);
   ```
   
   Do you expect it to do full or partial deserialization? I would say that as 
`deserializer` here is constructed via a constructor that does partial 
deserialization, I would expect it to only do partial deserializations.




-- 
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]


Reply via email to