5

I have a for-statement in java-7 and it's work fine:

Character cha = new Character(',');
String ncourseIds = null;
String pastCourseIds = null;
for (EquivalentCourse equivalentCourse : equivalentCourses) {
  if(equivalentCourse.getNcourse() != null){
    ncourseIds += equivalentCourse.getNcourse().getId()+ ",";   
  } else if(equivalentCourse.getPastCourse() != null) {
    pastCourseIds +=equivalentCourse.getPastCourse().getId()+","; 
  }
}
if(!ncourseIds.isEmpty() &&cha.equals(ncourseIds.charAt(ncourseIds.length()-1))) {
  ncourseIds = ncourseIds.substring(0, ncourseIds.length()-1);
}
if(!pastCourseIds.isEmpty()&& cha.equals(pastCourseIds.charAt(pastCourseIds.length()-1))) {
  pastCourseIds = pastCourseIds.substring(0,pastCourseIds.length()-1);
}

Now I want to convert my code to Stream & collect in java-8, I implement half of my business about filter not null Ncourse:

equivalentCourses.stream().filter(obj -> obj.getNcourse() != null )
                 .map(obj -> obj.getNcourse().getId()).collect(Collectors.joining(",")); 

but I don't know to implement it's else-statement. any help?

2
  • 4
    Why do you use Character instead of char? This makes your code harder to read and wastes resources. Though, you don’t need it at all, if you replace !ncourseIds.isEmpty() && ha.equals(ncourseIds.charAt(ncourseIds.length()-1)) with a simple ncourseIds.endsWith(",") and likewise, !pastCourseIds.isEmpty() && cha.equals(pastCourseIds.charAt(pastCourseIds.length()-1)) with pastCourseIds.endsWith(","). To collect two strings with streams, you can simple perform two stream operations. Commented Feb 15, 2017 at 13:40
  • 3
    @Holger already said it: use two stream operations (one for getNcourse() != null and one for getNcourse() == null && getPastCourse() != null). Commented Feb 15, 2017 at 13:57

3 Answers 3

6

As a stream call chain is complex make two streams - avoiding the conditional branches.

String ncourseIds = equivalentCourses.stream()
   .filter(equivalentCourse -> equivalentCourse.getNcourse() != null)
   .map(EquivalentCourse::getNcourse)
   .map(x -> String.valueOf(x.getId()))
   .collect(Collectors.joining(", "));

String pastCourseIds = equivalentCourses.stream()
   .filter(equivalentCourse -> equivalentCourse.getNcourse() == null
          && equivalentCourse.getPastCourse() != null)
   .map(EquivalentCourse::getPastCourse)
   .map(x -> String.valueOf(x.getId()))
   .collect(Collectors.joining(", "));

This also is code focusing on the resulting two strings, with an efficient joining.

By the way, if this is for an SQL string, you may use a PreparedStatement with an Array.


Embellishment as commented by @Holger:

String ncourseIds = equivalentCourses.stream()
   .map(EquivalentCourse::getNcourse)
   .filter(Objects::nonNull)
   .map(NCourse::getId)
   .map(String::valueOf)
   .collect(Collectors.joining(", "));

String pastCourseIds = equivalentCourses.stream()
   .filter(equivalentCourse -> equivalentCourse.getNcourse() == null)
   .map(EquivalentCourse::getPastCourse)
   .filter(Objects::nonNull)
   .map(EquivalentCourse::getPastCourse)
   .map(PastCourse::getId)
   .map(String::valueOf)
   .collect(Collectors.joining(", "));
Sign up to request clarification or add additional context in comments.

6 Comments

Nice use of the double-colon!
You can simplify by changing the order. First stream op: .map(EquivalentCourse::getNcourse) .filter(Objects::nonNull), second stream op: .filter(equivalentCourse -> equivalentCourse.getNcourse() == null) .map(EquivalentCourse::getPastCourse) .filter(Objects::nonNull).
@Holger you are right, and also the getId could be improved with some class knowledge
@JoopEggen If equivalentCourses has huge number of elements, this is inefficient. Isn't? If so, is there any way to make it efficiently?
@Gibbs no, neither in O-complexity, neither in speed or space usage. In an old for-loop one could hold two StringBuilders and append to them in one loop and an internal if-else for appending to either StringBuilder. Making that two for-loops for each StringBuilder would be somewhat the same.
|
1

You could group by condition and then remap:

public void booleanGrouping() throws Exception {
    List<String> strings = new ArrayList<>();
    strings.add("ala");
    strings.add("ela");
    strings.add("jan");

    strings.stream()
            .collect(
                    Collectors.groupingBy(s -> s.endsWith("a")) // using function Obj -> Bool not predicate
            ).entrySet()
            .stream()
            .collect(
                    Collectors.toMap(
                            e -> e.getKey() ? "Present" : "Past",
                            e -> e.getValue().stream().collect(Collectors.joining(""))
                    )
            );
}

First stream group by condition, you should use equivalentCourse.getNcourse() != null second remap collections from value to string. You could introduce:

enum PresentPast{
    Present, Past
    PresentPast is(boolean v){
         return v ? Present : Past
    }
}

and change e -> e.getKey() ? "Present" : "Past" to enum based solution.

Edit:

Solution for else if:

public Map<Classifier, String> booleanGrouping() throws Exception {
    List<String> strings = new ArrayList<>();
    strings.add("ala");
    strings.add("ela");
    strings.add("jan");
    // our ifs:
    /*
        if(!string.endsWith("n")){
        }else if(string.startsWith("e")){}

        final map should contains two elements
        endsWithN -> ["jan"]
        startsWithE -> ["ela"]
        NOT_MATCH -> ["ala"]

     */
    return strings.stream()
            .collect(
                    Collectors.groupingBy(Classifier::apply) // using function Obj -> Bool not predicate
            ).entrySet()
            .stream()
            .collect(
                    Collectors.toMap(
                            e -> e.getKey(),
                            e -> e.getValue().stream().collect(Collectors.joining(""))
                    )
            );
}

enum Classifier implements Predicate<String> {
    ENDS_WITH_N {
        @Override
        public boolean test(String s) {
            return s.endsWith("n");
        }
    },
    STARTS_WITH_E {
        @Override
        public boolean test(String s) {
            return s.startsWith("e");
        }
    }, NOT_MATCH {
        @Override
        public boolean test(String s) {
            return false;
        }
    };

    public static Classifier apply(String s) {
        return Arrays.stream(Classifier.values())
                .filter(c -> c.test(s))
                .findFirst().orElse(NOT_MATCH);
    }
}

2 Comments

That is good solution only for if-else statement, but question is about else-if statement. In that case you need to perform two stream operation.
I see :) I will fix that (you need introduce a little bit different condition) when I back to my main PC :D
0

Update

To add an alternative, here's what the code would look like to do the work with two filter() operations. Note, this has the impact of iterating over the entire collection a second time, which may have a performance impact if this is a large collection.

I also went ahead and simplified some of the logic in regards to the string joining. Correct me if I missed anything there.

final List<String> courseIdList = new ArrayList<>();
final List<String> pastCourseIdList = new ArrayList<>();

equivalentCourses.stream().filter((current) -> current.getNcourse() != null)
                .forEach((current) -> courseIdList.add(current.getNcourse().getId()));

equivalentCourses.stream().filter((current) -> current.getNcourse() != null && current.getPastCourse() != null)
                .forEach((current) -> pastCourseIdList.add(current.getPastCourse().getId()));

String ncourseIds = String.join(",", courseIdList);
String pastCourseIds = String.join(",", pastCourseIdList);

Original answer

For your use case, it may make the most sense to use the forEach() lambda. This will be the easiest way to do the translation.

java.lang.Character cha = new java.lang.Character(',');

final StringBuilder ncourseIdBuilder = new StringBuilder();
final StringBuilder pastCourseIdBuilder = new StringBuilder();
equivalentCourses.stream().forEach((equivalentCourse) -> {
    if (equivalentCourse.getNcourse() != null) {
        ncourseIdBuilder.append(equivalentCourse.getNcourse().getId()).append(",");
    } else if (equivalentCourse.getPastCourse() != null) {
        pastCourseIdBuilder.append(equivalentCourse.getPastCourse().getId()).append(",");
    }
});

String ncourseIds = ncourseIdBuilder.toString();
String pastCourseIds = pastCourseIdBuilder.toString();

if (!ncourseIds.isEmpty() && cha.equals(ncourseIds.charAt(ncourseIds.length() - 1))) {
    ncourseIds = ncourseIds.substring(0, ncourseIds.length() - 1);
}
if (!pastCourseIds.isEmpty() && cha.equals(pastCourseIds.charAt(pastCourseIds.length() - 1))) {
    pastCourseIds = pastCourseIds.substring(0, pastCourseIds.length() - 1);
}

You can rewrite the code using filter() expressions, but it'll require a bigger re-working of the logic in the conditionals, which introduces the risk you might break something if this isn't tested well. The logic changes are exactly what @Holger and @Ole V.V. reference in their comments to the original question.

Whether you use forEach() or the filters, lambdas cannot access non-final variables within the expression, hence why I added the final StringBuilder variable outside the scope of the loop.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.