You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2021/06/20 04:19:02 UTC

[GitHub] [dolphinscheduler] ruanwenjun commented on a change in pull request #5666: [Improvement][API] Simplify the Check of Result by introducing several new methods.

ruanwenjun commented on a change in pull request #5666:
URL: https://github.com/apache/dolphinscheduler/pull/5666#discussion_r654873827



##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/utils/Result.java
##########
@@ -72,6 +72,18 @@ private Result(Status status) {
         return new Result<>(data);
     }
 
+    public boolean isSuccess() {

Review comment:
       Hi, to be honest, I don't recommend modifying this class, this may be related to programming habits :).
   I think this class should be kept as simple as possible, he shouldn't care about the status judgements.
   Another point is that if we add more status judgements in the future, we need to modify this class again.
   For example, if we add a new status of result called unknown, then we need to modify this class and add a new method, am I right? 
   
   But you are right, right now there is a lot of duplicate code in many placements, once we need to change the rule of  status judgement, we need to modify many placements. I may prefer to use a new class to take care of the status judgement, rather than modified the current class.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org