I want to overwrite my objects status value if a corresponding key/value pair can be found in my HashMap.

Sample Model:

public class Url {
  private String url;
  private String status;
}

private List<Url> overwriteStatus(List<Url> myObjects) {
   final var myChecklist = Map.of("www.foo.com", "blocked");

  //if the key exists in `myChecklist`, then overwrite the status
  myObjects.stream().forEach(item -> Optional.ofNullable(myChecklist.get(item.getUrl())).ifPresent(item::setStatus));

  return myObjects;
}

My current approach with Optionals feels very messy.

What is the best approach to check if the value exists in a HashMap, and if so, use that value in a next step?

CodePudding user response：

Documentation for the Stream API warns't against the usage of stateful streams.

Functions used in streams has to be pure, i.e. don't cause mutations of the stream elements, don't modify objects outside the stream (that's what basically happens in your code).

Your task can be fulfilled with streams without violating the guidelines mentioned above. For that, instead of changing the state if a new status has to be applied, a new object needs to be created.

I also suggest to Url objects immutable. So that the same instances could be safely reused in different parts of the application.

private List<Url> overwriteStatus(List<Url> myObjects, 
                                  Map<String, String> myChecklist) {

    return myObjects.stream()
            .map(item -> !myChecklist.containsKey(item.getUrl()) ? item :
                    new Url(item.getUrl(), myChecklist.get(item.getUrl())))
            .collect(Collectors.toList());
}

Immutable class Url (no setters, all fields are private)

public class Url {
    private final String url;
    private final String status;

    // constructor and getters
}

Note:

• With regard to the Optional type, it was introduced in the JDK for only one particular purpose: to represent the return type of methods that could potentially yield null and that's it. The practice of utilizing the Optional just in order to chaining methods on it is considered to be an antipattern.

CodePudding user response：

I don't think you should use a Stream since you are modifying the objects as you are iterating the collection.

Just use a loop

for (Url item : myObjects) {
  String status = myCheckList.get(item.getUrl());
  if (status == null) continue;
  u.setStatus(status);
}

CodePudding user response：

Use filter, then forEach. I think this is the better choice.

No problem with using a Stream for this use case. It is certainly not "stateful" as I read.

private List<Url> overwriteStatus(List<Url> myObjects) {
    final var myChecklist = Map.of("www.foo.com", "blocked");

    // if the key exists in `myChecklist`, then overwrite the status
    myObjects.entrySet()
             .stream()
             .filter(entry -> myCheckList.containsKey(entry.getUrl))
             .map(Map.Entry::getValue)
             .forEach(item -> item.setStatus(status));

    return myObjects;
}