-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39303/#review102644
-----------------------------------------------------------



gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionManager.java
 (line 2501)
<https://reviews.apache.org/r/39303/#comment160393>

    I really like this new code. The diff is so much easier to review!
    
    One more idea. I didn't like that every time we call handleMemberEvent we 
create a new EventHandler object. It doesn't have any state on it so we could 
just have canonical instances of it.
    
    But even better is to put the handleEvent methods that you have on 
EventHandler on MemberEvent. And the override code on the subclasses of 
MemberEvent. Then you can get rid of this switch statement and just call 
ev.handleEvent(...). You will need to pass DistributedManager to this method so 
it can get the membershipListeners and allMembershipListeners and 
closeInProgress and logger.
    
    This seems more object oriented and will produce less garbage.


- Darrel Schneider


On Oct. 13, 2015, 4:53 p.m., Barry Oglesby wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39303/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2015, 4:53 p.m.)
> 
> 
> Review request for geode.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Added exception handling for all callbacks
> Refactored handler code
> 
> 
> Diffs
> -----
> 
>   
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionManager.java
>  36d160248647b5a25bf1e82e8d7709dfb84d0261 
> 
> Diff: https://reviews.apache.org/r/39303/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Barry Oglesby
> 
>

Reply via email to