trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sureshsubbiah <...@git.apache.org>
Subject [GitHub] incubator-trafodion pull request #697: [TRAFODION-2218] Memory leak from JVM...
Date Wed, 14 Sep 2016 14:32:25 GMT
Github user sureshsubbiah commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/697#discussion_r78760345
  
    --- Diff: core/sql/src/main/java/org/trafodion/sql/udr/LmUDRObjMethodInvoke.java ---
    @@ -68,7 +68,7 @@ static LmUDRObjMethodInvoke makeNewObj(
             return result;
         }
     
    -    public static class ReturnInfo
    +    public class ReturnInfo
    --- End diff --
    
    @zellerh, I agree that this change is not strictly necessary for this JIRA. I have tried
to explain my motivation for making it in the comment section of https://issues.apache.org/jira/browse/TRAFODION-2218
    
    The way I see it here are the pros and cons of Return being a static class
    Pro
    1) Can be instantiated without an instance of the outer class. Having it be a nested class
provides better encapsulation.
    
    Cons
    1) It was not clear to me when I made the change when the static class will be garbage
collected. Now I read this link and see that I do not need to be concerned with GC. http://stackoverflow.com/questions/17553218/when-is-static-nested-class-gets-garbage-collected
. Static variables could get in the way of GC I suppose? So this is not really a con.
    
    2) The static class is not accessing any static data members of the enclosing class. The
static class is also not instantiated outside the enclosing class. There seems to be no programmatic
rule broken by having the class be non-static.
    
    I can roll this change back if that is preferable. My main concern with GC has been shown
to be a non-issue. At this point I suppose use of static modifier here is programming preference.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message