patternjavaMinor
Passing data from one model to another
Viewed 0 times
passingoneanotherfromdatamodel
Problem
In my task I have to pass some data from one model to another, where each model contains lists:
However, while I think it looks awful, I don't have any other ideas as to making it more elegant. Another issue is about the size of the method: Would it be better to split it into several, smaller
List vProcesses = new ArrayList();
for (Process process : processList) {
VisualProcess vProcess = new VisualProcess();
vProcess.setId(process.getId());
List steps = new ArrayList();
List connections = new ArrayList();
for(ProcessStep processStep : process.getSteps()){
VisualProcessStep vps = new VisualProcessStep();
vps.setActivityId(processStep.getActivityId());
vps.setDefinition(processStep.getDefinition());
vps.setId(processStep.getId());
vps.setParameters(processStep.getParameters());
vps.setStatus(processStep.getStatus());
for (Activity activity : process.getActivities()) {
if (activity.getId().equals(vps.getActivityId())){
vps.setActivityName(activity.getName());
vps.setActivityRealization(activity.getRealization());
break;
}
}
steps.add(vps);
for (ProcessStep nextProcessStep : processStep.getNextSteps()) {
VisualProcessStepConnection vpsc = new VisualProcessStepConnection();
vpsc.setFrom(processStep.getId());
vpsc.setTo(nextProcessStep.getId());
connections.add(vpsc);
}
}
vProcess.setSteps(steps);
vProcess.setConnections(connections);
vProcesses.add(vProcess);
}However, while I think it looks awful, I don't have any other ideas as to making it more elegant. Another issue is about the size of the method: Would it be better to split it into several, smaller
Solution
Is it too awful code?
Not too awful and not even bad, but the huge wall of code looks ugly.
There are a few minor issues like missing blank after "for" in
and before "{" in
but that's something your IDE can do for you (Ctrl-Shift-F in Eclipse).
Otherwise, all you need is to extract a few methods like
This can be repeated many times and you surely shouldn't extract everything. When the methods don't get used elsewhere, this is very subjective as you're trading a too long method for too many methods.
The sweet spot are methods somewhere about 15 lines long. As all the methods do is just copying everything and there isn't much to think about, they may be a bit longer. I'd personally split it into probably 3 methods.
Obviously, there may be a smarter way avoiding the manual copying. It surely can be done via reflection, but reflection is verbose and slow and ugly itself, so I wouldn't consider it for this task.
Maybe you could create a constructor (or a factory method) for
Not too awful and not even bad, but the huge wall of code looks ugly.
There are a few minor issues like missing blank after "for" in
for(ProcessStep processStep : process.getSteps()){and before "{" in
if (activity.getId().equals(vps.getActivityId())){but that's something your IDE can do for you (Ctrl-Shift-F in Eclipse).
Otherwise, all you need is to extract a few methods like
private VisualProcessStep toVisualProcessStep(Process process, ProcessStep processStep) {
VisualProcessStep result = new VisualProcessStep();
result.setActivityId(processStep.getActivityId());
result.setDefinition(processStep.getDefinition());
result.setId(processStep.getId());
result.setParameters(processStep.getParameters());
result.setStatus(processStep.getStatus());
for (Activity activity : process.getActivities()) {
if (activity.getId().equals(result.getActivityId())) {
result.setActivityName(activity.getName());
result.setActivityRealization(activity.getRealization());
break;
}
}
return result;
}This can be repeated many times and you surely shouldn't extract everything. When the methods don't get used elsewhere, this is very subjective as you're trading a too long method for too many methods.
The sweet spot are methods somewhere about 15 lines long. As all the methods do is just copying everything and there isn't much to think about, they may be a bit longer. I'd personally split it into probably 3 methods.
Obviously, there may be a smarter way avoiding the manual copying. It surely can be done via reflection, but reflection is verbose and slow and ugly itself, so I wouldn't consider it for this task.
Maybe you could create a constructor (or a factory method) for
VisualProcessStep?Code Snippets
for(ProcessStep processStep : process.getSteps()){if (activity.getId().equals(vps.getActivityId())){private VisualProcessStep toVisualProcessStep(Process process, ProcessStep processStep) {
VisualProcessStep result = new VisualProcessStep();
result.setActivityId(processStep.getActivityId());
result.setDefinition(processStep.getDefinition());
result.setId(processStep.getId());
result.setParameters(processStep.getParameters());
result.setStatus(processStep.getStatus());
for (Activity activity : process.getActivities()) {
if (activity.getId().equals(result.getActivityId())) {
result.setActivityName(activity.getName());
result.setActivityRealization(activity.getRealization());
break;
}
}
return result;
}Context
StackExchange Code Review Q#95022, answer score: 3
Revisions (0)
No revisions yet.