CODEWITHSUNDEEP
javaif-statementconditional-statements
Pekka
Pekka

Reputation: 171

Good Design for if else object mapping

I have this logic:

public void method(Boo result, Foo foo, Bar bar) {
   if(foo != null) {
      if(foo.getId() != null){
         result.setId(foo.getId());
      } else {
         result.setId(bar.getId());
      }
      if(foo.getName() != null){
         boo.setName(foo.getName());
      } else {
         result.setName(bar.getName());
      }
      // and many similar attributes
   } else {
      result.setId(bar.getId());
      result.setName(bar.getName());
      // and many similar attributes
   }
}

I find this way ugly, is there any way to make it in better design. I know that is better to use mapstruct, but in this project I can't.

Upvotes: 0

Views: 666

Answers (6)

Loris Securo
Loris Securo

Reputation: 7638

With common class/interface:

If Foo and Bar extend a common base class/interface you could create a generic method to apply the logic to any attribute.

For example lets say that they implement this interface:

public interface IBaseInterface {
    Long getId();
    String getName();
}

Then we could create this generic method:

public static <T> T getAttribute(IBaseInterface foo, IBaseInterface bar, Function<IBaseInterface, T> function) {
    if(foo != null) {
        T attribute = function.apply(foo);
        if(attribute != null) {
            return attribute;
        }
    }
    return function.apply(bar);
}

And use it like this:

result.setId(getAttribute(foo, bar, IBaseInterface::getId));
result.setName(getAttribute(foo, bar, IBaseInterface::getName));

Without common class/interface:

If they don't extend a common base class/interface you could still use this technique but you'd have to pass another parameter to the method:

public static <T> T getAttribute(Foo foo, Bar bar, Function<Foo, T> functionFoo, Function<Bar, T> functionBar) {
    if(foo != null) {
        T attribute = functionFoo.apply(foo);
        if(attribute != null) {
            return attribute;
        }
    }
    return functionBar.apply(bar);
}

And use it like this:

result.setId(getAttribute(foo, bar, Foo::getId, Bar::getId));
result.setName(getAttribute(foo, bar, Foo::getName, Bar::getName));

Upvotes: 1

Loris Securo
Loris Securo

Reputation: 7638

By having foo as an Optional you could simplify your code like this:

Optional<Foo> optionalFoo = Optional.ofNullable(foo);
    
result.setId(optionalFoo.map(Foo::getId).orElseGet(bar::getId));
result.setName(optionalFoo.map(Foo::getName).orElseGet(bar::getName));
// ...

Upvotes: 1

Pekka
Pekka

Reputation: 171

I avoid the if else using ObjectUtils::firstNonNull as bellow:

public void method(Boo result, Foo foo, Bar bar) {
   if(foo != null) {
      result.setId(ObjectUtils.firstNonNull(foo.getId(), bar.getId()));
      result.setId(ObjectUtils.firstNonNull(foo.getName(), bar.getName()));
      // and many similar attributes
   } else {
      result.setId(bar.getId());
      result.setName(bar.getName());
      // and many similar attributes
   }
}

I'm open for other suggests!

Upvotes: 1

Soni
Soni

Reputation: 152

If the mapping objects are having same properties, and if you have spring beans in your project dependency, BeanUtils.copyProperties(source, destination);

Upvotes: 0

Alex Mandelias
Alex Mandelias

Reputation: 517

if statements don't need curly brackets {} when there there is a single statement afterwards. Think of it as {...} as a single statement wrapping many other inside it.
Also Java doesn't really care about whitespaces and indentation so you can put multiple statements in the same line.

With this knowledge you can make the code prettier like this:

public void method(Boo result, Foo foo, Bar bar) {
    if (foo != null) {
        if (foo.getId() != null) result.setId(foo.getId());
        else result.setId(bar.getId());

        if (foo.getName() != null) boo.setName(foo.getName());
        else result.setName(bar.getName());

    } else {
        result.setId(bar.getId());
        result.setName(bar.getName());
   }
}

or at the very least remove the {}:

public void method(Boo result, Foo foo, Bar bar) {
    if (foo != null) {
        if (foo.getId() != null)
            result.setId(foo.getId());
        else 
            result.setId(bar.getId());

        if (foo.getName() != null)
            boo.setName(foo.getName());
        else 
            esult.setName(bar.getName());

    } else {
        result.setId(bar.getId());
        result.setName(bar.getName());
   }
}

Upvotes: 0

Gilbert Le Blanc
Gilbert Le Blanc

Reputation: 51565

I'd switch the tests around. It's a little cleaner, but it's still going to look messy.

public void method(Boo result, Foo foo, Bar bar) {
    setResult(result, bar);
    if (foo != null) { setResult(result, foo); }
}

private void setResult(Boo result, Bar bar) {
    result.setId(bar.getId());
    ...
}

private void setResult(Boo result, Foo foo) {
    if (foo.getId != null) { result.setId(foo.getId()); }
    ...
}

Upvotes: 2

Related Questions