0
\$\begingroup\$

I have the following code, and I'd like to refactor it to a more functional way:

public void processPersons(List<Person> personList) {
    for (Person person : personList) {
        Integer addressId = createAddress(person);
        if (addressId != null) {
            updateDbStatus(addressId, person);
        }
    }
}

How do I convert the above to a more functional style of programming?

\$\endgroup\$
13
  • \$\begingroup\$ Use method references to shorten your map() \$\endgroup\$ Jul 20, 2017 at 9:11
  • \$\begingroup\$ Why is this off-topic? \$\endgroup\$
    – Erik Pragt
    Jul 21, 2017 at 3:41
  • 1
    \$\begingroup\$ Well, I can remove that part, and say: "how can I refactor this code into functional code?". The code is working in my example, that's the first part of the code. The second part is there to show that I tried something, and didn't just put some code here for others to refactor. I had a similar question on SO, and they pointed me to CodeReview. It's quite ironic that now you point me back. \$\endgroup\$
    – Erik Pragt
    Jul 22, 2017 at 4:49
  • 2
    \$\begingroup\$ @JudeNiroshan That seems to have been an attempt at refactoring the code, but the attempt failed. That doesn't make the original code non-working or off-topic. \$\endgroup\$ Jul 22, 2017 at 9:13
  • 1
    \$\begingroup\$ You can probably rewrite your code like this personList.forEach(person -> Optional.ofNullable(createAddress(person)).ifPresent(addressId -> updateDbStatus(addressId, person))); , but I wouldn't recommend it. The code you have now is better in my opinion. \$\endgroup\$ Jul 22, 2017 at 9:16

2 Answers 2

3
\$\begingroup\$

One of variants is to have PersonWithAddress subclass with created addressIp:

    persons
            .stream()
            .map(PersonWithAddress::new)
            .forEach(this::updateDbStatus);

You call updateDbStatus where check the address for null:

void updateDbStatus(PersonWithAddress personWithAddress) {
    if (personWithAddress.getAddressIp() != null) {
        // do the update
    }
}

Model classes like these ones:

class Person{
    Person(final Person original) {
        // copy constructor
    }
}

class PersonWithAddress extends Person {
    private final Integer addressIp;

    PersonWithAddress(final Person person) {
        super(person);
        this.addressIp = createAddress();
    }

    Integer getAddressIp() {
        return addressIp;
    }

    private Integer createAddress() {
        // have a logic to create an address
        return 0;
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for that. I had something similar (I just used a Pair/Tuple2), but I didn't think the end result was better than the code already in place. \$\endgroup\$
    – Erik Pragt
    Jul 21, 2017 at 3:43
  • \$\begingroup\$ @ErikPragt That might be an indication that the code you currently have isn't that bad after all. \$\endgroup\$ Jul 22, 2017 at 9:11
  • \$\begingroup\$ @ErikPragt this answer is most helpful, so flag it. beside that we can't see your answer yet! \$\endgroup\$
    – Sergii
    Aug 1, 2017 at 5:34
0
\$\begingroup\$

As you are working with Java 8, you'd be happy to explore the other features in it like Optional<T>

You can change your createAddress() to return an Optional which will be more expressive.

Optional<Person> createAddress(person){
   //write your logic and return the result wrapped in an Optional
}

As addressId has one-to-one mapping with Person, you should add an identifier to the Person class to hold the addressId. Then you have to modify the method signature of updateDbStatus as updateDbStatus(Person person)

class SomeClass{

 void someMethod(){

   personList.stream()
    .map(SomeClass::createAddress)
    .ifPresent(SomeClass::updateDbStatus)
 }

}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks Jude, but this won't really fix the issue, because the updatDbStatus still needs the person and the addressId. There is no one-to-one mapping between these classes. \$\endgroup\$
    – Erik Pragt
    Jul 20, 2017 at 10:06

Not the answer you're looking for? Browse other questions tagged or ask your own question.