HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Passing data from one model to another

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
passingoneanotherfromdatamodel

Problem

In my task I have to pass some data from one model to another, where each model contains lists:

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

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.