llllkid commented on code in PR #16939:
URL: 
https://github.com/apache/dolphinscheduler/pull/16939#discussion_r1910022198


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkflowDefinitionServiceImpl.java:
##########
@@ -2118,37 +2118,38 @@ protected void doBatchOperateWorkflowDefinition(User 
loginUser,
                 List<TaskDefinitionLog> taskDefinitionLogs =
                         
taskDefinitionLogDao.queryTaskDefineLogList(workflowTaskRelations);
                 Map<Long, Long> taskCodeMap = new HashMap<>();
-                for (TaskDefinitionLog taskDefinitionLog : taskDefinitionLogs) 
{
+                taskDefinitionLogs.forEach(taskDefinitionLog -> {
                     try {
-                        long taskCode = CodeGenerateUtils.genCode();
-                        taskCodeMap.put(taskDefinitionLog.getCode(), taskCode);
-                        taskDefinitionLog.setCode(taskCode);
-                        if 
(TaskTypeUtils.isSwitchTask(taskDefinitionLog.getTaskType())) {
-                            final String taskParams = 
taskDefinitionLog.getTaskParams();
-                            final SwitchParameters switchParameters =
-                                    JSONUtils.parseObject(taskParams, 
SwitchParameters.class);
-                            if (switchParameters == null) {
-                                throw new IllegalArgumentException(
-                                        "Switch task params: " + taskParams + 
" is invalid.");
-                            }
-                            SwitchParameters.SwitchResult switchResult = 
switchParameters.getSwitchResult();
-                            
switchResult.getDependTaskList().forEach(switchResultVo -> {
-                                
switchResultVo.setNextNode(taskCodeMap.get(switchResultVo.getNextNode()));
-                            });
-                            if (switchResult.getNextNode() != null) {
-                                switchResult.setNextNode(
-                                        
taskCodeMap.get(switchResult.getNextNode()));
-                            }
-                            
taskDefinitionLog.setTaskParams(JSONUtils.toJsonString(switchParameters));
-                        }
+                        taskCodeMap.put(taskDefinitionLog.getCode(), 
CodeGenerateUtils.genCode());
                     } catch (CodeGenerateException e) {
                         log.error("Generate task definition code error, 
projectCode:{}.", targetProjectCode, e);
                         putMsg(result, Status.INTERNAL_SERVER_ERROR_ARGS);
                         throw new 
ServiceException(Status.INTERNAL_SERVER_ERROR_ARGS);
                     }
+                });
+                for (TaskDefinitionLog taskDefinitionLog : taskDefinitionLogs) 
{
+                    
taskDefinitionLog.setCode(taskCodeMap.get(taskDefinitionLog.getCode()));
                     taskDefinitionLog.setProjectCode(targetProjectCode);
                     taskDefinitionLog.setVersion(0);
                     taskDefinitionLog.setName(taskDefinitionLog.getName());
+                    if 
(TaskTypeUtils.isSwitchTask(taskDefinitionLog.getTaskType())) {
+                        final String taskParams = 
taskDefinitionLog.getTaskParams();
+                        final SwitchParameters switchParameters =
+                                JSONUtils.parseObject(taskParams, 
SwitchParameters.class);
+                        if (switchParameters == null) {
+                            throw new IllegalArgumentException(
+                                    "Switch task params: " + taskParams + " is 
invalid.");
+                        }
+                        SwitchParameters.SwitchResult switchResult = 
switchParameters.getSwitchResult();
+                        
switchResult.getDependTaskList().forEach(switchResultVo -> {
+                            
switchResultVo.setNextNode(taskCodeMap.get(switchResultVo.getNextNode()));
+                        });
+                        if (switchResult.getNextNode() != null) {
+                            switchResult.setNextNode(
+                                    
taskCodeMap.get(switchResult.getNextNode()));
+                        }
+                        
taskDefinitionLog.setTaskParams(JSONUtils.toJsonString(switchParameters));
+                    }

Review Comment:
   > > The commit shows it's not a display issue. 
https://github.com/apache/dolphinscheduler/blob/48304f2a6c20de7471739b8889c2d02c41eef17d/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/WorkflowDefinitionServiceImpl.java
   > 
   > I think I misunderstood your question. Do you want to ask me why I move 
this code after `taskDefinitionLog.setName`? Because for better readability:
   > 
   > ```
   >  taskDefinitionLog.setCode();
   >  taskDefinitionLog.setProjectCode();
   >  taskDefinitionLog.setVersion();
   >  taskDefinitionLog.setName();
   >  if (TaskTypeUtils.isSwitchTask(taskDefinitionLog.getTaskType())) {
   >  ...
   >       taskDefinitionLog.setTaskParams();
   >  ...
   >  }
   > ```
   > 
   > Is better than
   > 
   > ```
   >  taskDefinitionLog.setCode();
   >  if (TaskTypeUtils.isSwitchTask(taskDefinitionLog.getTaskType())) {
   >  ...
   >       taskDefinitionLog.setTaskParams();
   >  ...
   >  }
   >  taskDefinitionLog.setProjectCode();
   >  taskDefinitionLog.setVersion();
   >  taskDefinitionLog.setName();
   > ```
   
   Is that the question you want to ask? @SbloodyS 



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to