0

I have a request from tech lead to rewrite this code and replace for-loop with generic lambda. I doubt that this will lead to more simpler, more readable and maintainable code.

Is there a really a good way to do that, please?

The question is about how to transform current for-loop into the lambda function. Change of item's data structure is completely out of scope. See the loop - it is a devision of the states list while simultaneously checking value in addressType at the same index.

How to do that with lambda and will it actually simplify the code?

List<String> states = Arrays.asList(item.getState().split(","));
List<String> addressType = Arrays.asList(item.getAddressType().split(","));
List<String> mailingStates = new ArrayList<>();
List<String> physicalStates = new ArrayList<>();


for(int i = 0; i<states.size(); i++){
    if(Constants.MAILING.equals(addressType.get(i))){
        mailingStates.add(states.get(i));
    } else {
        physicalStates.add(states.get(i));
    }
}

Need to say - Java 8 only

Gautham M
  • 4,816
  • 3
  • 15
  • 37
AlexR
  • 9
  • 1
  • 6
  • This is a re-post of https://stackoverflow.com/q/68210605/5221149 – Andreas Jul 01 '21 at 15:04
  • "*See the loop - it is a devision of the states list while simultaneously checking value in addressType at the same index.*" so, you just want to walk both lists at the same time? Seems pretty straight forward - make a stream that goes through an item from each of the two lists. Perhaps a `Stream.generate()` or `Stream.iterate()` using the iterators of both lists suffice. Then you just need to consume pairs of elements. – VLAZ Jul 01 '21 at 15:07
  • Had a look, seems there isn't a straight up built-in zip operation but [here](https://stackoverflow.com/questions/17640754/zipping-streams-using-jdk8-with-lambda-java-util-stream-streams-zip) is how it can be implemented. "zip" is the generic name of going through two or more lists and similar. Usually it's a function that accepts a lambda and a bunch of lists/arrays/streams/and other similar things. – VLAZ Jul 01 '21 at 15:10

3 Answers3

0

The code will be the same, so I don't know what the point would be, but it will use a lambda expression block.

List<String> states = Arrays.asList(item.getState().split(","));
List<String> addressType = Arrays.asList(item.getAddressType().split(","));
List<String> mailingStates = new ArrayList<>(), physicalStates = new ArrayList<>();


        IntStream.range(0, states.size()).forEach(i -> {
            if (Constants.MAILING.equals(addressType.get(i))) {
                mailingStates.add(states.get(i));
            } else {
                physicalStates.add(states.get(i));
            }
        });
Andreas
  • 154,647
  • 11
  • 152
  • 247
  • Thank you Andreas, I agree with you , with this implementation usage of lambda makes no sense. – AlexR Jul 01 '21 at 15:17
0

What you want to do is go through two lists at the same name. The generic name for this operation is "zip" - when you go through two (or sometimes more) arrays/lists/streams/etc and do something with each element.

You can pick an implementation for streams from here: Zipping streams using JDK8 with lambda (java.util.stream.Streams.zip) there are many that are already implemented in existing libraries, as well. If you already have such a library in your project, you need but an import to use it.

For illustrative purposes, I'll assume there is an implementation available with this signature:

<A, B, C> Stream<C> zip(Stream<? extends A> a,
                        Stream<? extends B> b,
                        BiFunction<? super A, ? super B, ? extends C> zipper)

Also, a good simple generic utility would be a Pair class that has two values. There are many existing implementations. I'll an implementation with this this signature is available:

class Pair<LEFT, RIGHT> {
    Pair(LEFT left, RIGHT right);
    LEFT getLeft();
    RIGHT getRight();
}

This will hold the related state and address type. But you can also consider creating a specific object that encapsulates a given state and address type.

With these generic helpers, your code can look like this:

Stream<String> states = Arrays.stream(item.getState().split(","));
Stream<String> addressType = Arrays.stream(item.getAddressType().split(","));

Map<Boolean, List<String>> splitStates = zip(states, addressTypes, 
    (state, addressType) -> new Pair<String, String>(state, addressType))
    .collect(
      Collectors.partitioningBy(pair -> Constants.MAILING.equals(pair.getRight()), 
        collectors.mapping(pair -> pair.getLeft())
      )
    );

List<String> mailingStates = split.get(true);
List<String> physicalStates = split.get(false);

If lambdas are replaced with method references and some minor rearrangement when possible, then you get:

private static final Predicate<Pair<String, String> IS_Mailing = 
    pair -> Constants.MAILING.equals(pair.getRight());

/* ... */

Stream<String> states = Arrays.stream(item.getState().split(","));
Stream<String> addressType = Arrays.stream(item.getAddressType().split(","));

Map<Boolean, List<String>> splitStates = zip(states, addressTypes, Pair::new)
    .collect(Collectors.partitioningBy(IS_MAILING), 
        collectors.mapping(Pair::getLeft()));

List<String> mailingStates = split.get(true);
List<String> physicalStates = split.get(false);

And if instead of generic Pair class you implement a class like:

class StateData {
    private String state;
    private String addressType;

    public StateData(String state, String addressType) {
        this.state = state;
        this.addressType = addressType;
    }

    public String getState() { return this.state; } 
    public String getAddressType() { return this.addressType; } 
    public boolean isMailing() { 
        return Constants.MAILING.equals(this.getAddressType()); 
    }
}

The code becomes more semantic:

Stream<String> states = Arrays.stream(item.getState().split(","));
Stream<String> addressType = Arrays.stream(item.getAddressType().split(","));

Map<Boolean, List<String>> splitStates = zip(states, addressTypes, StateData::new)
    .collect(Collectors.partitioningBy(StateData::isMailing), 
        collectors.mapping(StateData::getState()));

List<String> mailingStates = split.get(true);
List<String> physicalStates = split.get(false);

One final consideration would be to create an enum for addressType instead of comparing to a constant.

VLAZ
  • 26,331
  • 9
  • 49
  • 67
0

You may use the partitioningBy to separate out items based on the Constants.MAILING.equals(addressType.get(i)) condition.

Map<Boolean, List<Integer>> map 
    = IntStream.range(0, states.size())
               .boxed()
               .collect(Collectors.partitioningBy(i -> Constants.MAILING.equals(addressType.get(i))));

List<String> mailingStates = map.get(true);
List<String> physicalStates = map.get(false);                             
Gautham M
  • 4,816
  • 3
  • 15
  • 37