Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/581#issuecomment-168013136
  
    Minor: How about extracting whole synchronized block to new private method?
    
    Actually setPropertyAndRestart() and restart() takes same logic except 
setting additional parameter and calling saveToFile().
    
    setPropertyAndRestart() can call saveToFile() safely when new method is 
executed without InterpreterException. So new private method doesn't need to 
take care about safeToFile().
    
    Next concern is that how we can unify logic whether two parameters are 
presented or not. 
    It would be achieved easily with null check, but someone can think it's not 
beautiful.
    So I'd like to hear our opinions about this suggestion.


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

Reply via email to