Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3792#discussion_r22305806
  
    --- Diff: 
network/common/src/main/java/org/apache/spark/network/util/JavaUtils.java ---
    @@ -41,6 +41,34 @@
     public class JavaUtils {
       private static final Logger logger = 
LoggerFactory.getLogger(JavaUtils.class);
     
    +  /** Deserialize a byte array using Java serialization. */
    +  public static <T> T deserialize(byte[] bytes) {
    +    try {
    +      ObjectInputStream is = new ObjectInputStream(new 
ByteArrayInputStream(bytes));
    +      Object out = is.readObject();
    +      is.close();
    +      return (T) out;
    +    } catch (ClassNotFoundException e) {
    +      throw new RuntimeException("Could not deserialize object", e);
    --- End diff --
    
    I was thinking that you don't expect to not have the class on hand... But 
sure, IllegalArgumentException because the bytes describe something invalid? 
The principle is to avoid RuntimeException since it is the superclass of all 
unchecked exceptions. If you ever wanted to catch this exception to deal with 
it you'd have no hope of distinguishing with a catch block. So reach for 
another standard and slightly more specific exception. Marginal argument here, 
but I think still common good practice in Java. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to