kennknowles commented on code in PR #37005:
URL: https://github.com/apache/beam/pull/37005#discussion_r2818320467


##########
sdks/java/extensions/protobuf/src/main/java/org/apache/beam/sdk/extensions/protobuf/ProtoCoder.java:
##########
@@ -293,23 +293,64 @@ protected ProtoCoder(Class<T> protoMessageClass, 
Set<Class<?>> extensionHostClas
     this.extensionHostClasses = extensionHostClasses;
   }
 
-  /** Get the memoized {@link Parser}, possibly initializing it lazily. */
+  /**
+   * Get the memoized {@link Parser}, possibly initializing it lazily. 
Attempts to use {@code
+   * getDefaultInstance()} first, falling back to instantiation if necessary, 
with clear error
+   * handling for unsupported cases.
+   */
   protected synchronized Parser<T> getParser() {
     if (memoizedParser == null) {
       try {
         if (DynamicMessage.class.equals(protoMessageClass)) {
           throw new IllegalArgumentException(
               "DynamicMessage is not supported by the ProtoCoder, use the 
DynamicProtoCoder.");
+        } else if (Message.class.equals(protoMessageClass)) {
+          throw new IllegalArgumentException(
+              "ProtoCoder does not support the raw Message interface. Use a 
concrete Protobuf-generated class.");
         } else {
-          @SuppressWarnings("unchecked")
-          T protoMessageInstance =
-              (T) 
protoMessageClass.getMethod("getDefaultInstance").invoke(null);
-          @SuppressWarnings("unchecked")
-          Parser<T> tParser = (Parser<T>) 
protoMessageInstance.getParserForType();
-          memoizedParser = tParser;
+          // Try using getDefaultInstance() first (preferred for generated 
Protobuf classes)
+          try {

Review Comment:
   This chain of try-catch can be cleaned up by having methods for each type of 
attempt, and returning early when one succeeds.
   
   ```java
   try {
     return getParserViaDefaultInstance(...)
   } catch (...) {
     // fall through
   }
   
   try {
     return getParserViaConstructor(...)
   } catch (...) {
     // fall through
   }
   ```
   
   This is valuable, because nested try-catch are the number one source of bugs 
in software dev.



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