Issue converting stringBuilder + forloop to stream + map

62 Views Asked by At
  • What I am trying to achieve:

I would like to convert a StringBuilder + forloop, like this:

public String question(List<MyPojo> myPojos) {
    final StringBuilder stringBuilder = new StringBuilder();
    for (MyPojo myPojo : myPojos) {
        try {
            String json = objectMapper.writeValueAsString(myPojo);
            stringBuilder.append(json).append("\n");
        } catch (JsonProcessingException e) {
                LOGGER.warn("bad");
        }
    }
    return stringBuilder.toString();
}

Into a stream() structure. This is not for performance, readability, or anything, I just want to try the conversion.

  • What did I try:
public String question(List<MyPojo> myPojos) {
    var stringBuilder = new StringBuilder();
    return myPojos.stream().map(onePojo -> delegate(onePojo, stringBuilder)).toString();
}

public StringBuilder delegate(MyPojo oneMyPojo, StringBuilder stringBuilder) {
    try {
        var json = objectMapper.writeValueAsString(oneMyPojo);
         stringBuilder.append(json).append("\n");
        } catch (JsonProcessingException e) {
                LOGGER.warn("bad");
    }
    return stringBuilder;
}
  • Issue

Unfortunately, the result string is not at all the same as the one provided by the for loop.

  • Question

What did I miss in my code?

2

There are 2 best solutions below

0
Youcef LAIDANI On BEST ANSWER

You don't need a StringBuilder at all in your example, you can simply use .collect(Collectors.joining("\n")) to join your strings, your can can be like this:

public String question(List<MyPojo> myPojos) {
    return myPojos.stream()
        .map(this::convertToJson) // Convert the object to json string
        .filter(json -> json != null && !json.isEmpty()) // retrieve non null or empty jsons
        .collect(Collectors.joining("\n")); // combine the result with backline separator
}

private String convertToJson(MyPojo myPojo) {
    try {
        return objectMapper.writeValueAsString(myPojo);
    } catch (JsonProcessingException e) {
        LOGGER.warn("bad");
        return "";
    }
}
0
Sweeper On

First, calling Stream.toString will not give you anything meaningful. It is very unlikely that the Stream implementation actually overrides toString to give you the elements of the stream all concatenated as strings. You should use .collectCollectors.joining("\n")) to join everything together.

Second, the function you passed to map modifies the existing stringBuilder. While you will probably get the expected result in stringBuilder if the whole stream is consumed, this is not how map is designed to be used. You are supposed to return the element that a given element should be transformed into, but the StringBuilder you are returning here contains all the results from the previous elements.

Third, the loop does not append anything to the string builder when an exception occurs, but Stream.map transforms every element. To implement this behaviour, you would need delegate to return a special value (like an empty string), and filter out that value after map. Alternatively, use flatMap(x -> delegate(x).stream()), where delegate returns an Optional<String>.

public String delegate(MyPojo oneMyPojo) {
    try {
        var json = objectMapper.writeValueAsString(oneMyPojo);
        return json;
    } catch (JsonProcessingException e) {
        LOGGER.warn("bad");
    }
    return "";
}

// ...

myPojos.stream()
    .map(onePojo -> delegate(onePojo)) // you can also use a method reference here
    .filter(json -> !json.isEmpty())
    .collect(Collectors.joining("\n"));

or

public Optional<String> delegate(MyPojo oneMyPojo) {
    try {
        var json = objectMapper.writeValueAsString(oneMyPojo);
        return Optional.of(json);
    } catch (JsonProcessingException e) {
        LOGGER.warn("bad");
    }
    return Optional.empty();
}

// ...

myPojos.stream()
    .flatMap(onePojo -> delegate(onePojo).stream())
    .collect(Collectors.joining("\n"));