4

I have a List<MyObject> , with 3 properties:

public class MyObject{
    long id;
    String active; 
    java.util.Date date;

    public MyObject(long id, String active, Date date) {
        this.id = id;
        this.active = active;
        this.date = date;
    }
}

(I know ... String hurts eyes). And this is a sample data which I'm working on:

ID  | ACTIVE | DATE
500925  1   2017-12-01 11:43:34
501145  1   2018-10-11 11:41:14
501146  1   2018-10-11 11:42:51
501147  1   2018-10-11 11:45:37

What I'm trying to do is setting all objects as active = 0 with a stream, except for the most recent , which I wanna keep as active.

I'm a bit stuck here, I can't match the correct way to do this:

myList.stream()
    .sorted(Comparator.comparing(MyObject::getDate))
    //what now?
;

The expected output should be:

ID  | ACTIVE | DATE
500925  0   2017-12-01 11:43:34
501145  0   2018-10-11 11:41:14
501146  0   2018-10-11 11:42:51
501147  1   2018-10-11 11:45:37

Thanks

Leviand
  • 2,745
  • 4
  • 29
  • 43

3 Answers3

5

EDIT: The approach shown here might be seen as an abuse of the Stream.map operation, even though in practice it will always work (with the caveat of the final note at the end of the initial version of the post).

Please refer to the addendum for a cleaner approach.


It's not necessary to do any sorting (which is a O(NlogN) operation), when all you have to do is find the element with the max date.

You could stream the list of MyObject elements, deactivate all of them, then find the element with the max date and finally activate it:

myList.stream()
    .map(myObject -> { myObject.setActive("0"); return myObject; })
    .max(Comparator.comparing(MyObject::getDate))
    .ifPresent(myObject -> myObject.setActive("1"));

This code would read much better if the MyObject class would have fluent methods to set the active property:

public MyObject activated() {
    active = "1";
    return this;
}

public MyObject deactivated() {
    active = "0";
    return this;
}

Now the solution would become:

myList.stream()
    .map(MyObject::deactivated)
    .max(Comparator.comparing(MyObject::getDate))
    .ifPresent(MyObject::activated);

Note: this approach might not work if the source of the stream reports itself as sorted. As you're using a list, this won't happen.


Addendum:

As discussed in the comments, the cleanest approach would be to set all the elements to inactive, then find the element with the max date and set it to active:

myList.forEach(myObject -> myObject.setActive("0"));
myList.stream()
    .max(Comparator.comparing(MyObject::getDate))
    .ifPresent(myObject -> myObject.setActive("1"));

Or using the fluent methods:

myList.forEach(MyObject::deactivated);
myList.stream()
    .max(Comparator.comparing(MyObject::getDate))
    .ifPresent(MyObject::activated);

Of course, this approach requires 2 passes over the list, but it clearly shows the intention of the developer and doesn't abuse the stream API.

A variant of this last snippet could be:

myList.forEach(MyObject::deactivated);
if (!myList.isEmpty()) 
        Collections.max(myList, Comparator.comparing(MyObject::getDate))
                .activated();
fps
  • 33,623
  • 8
  • 55
  • 110
  • 1
    I thought about this, but wanted to stay as close as I can to the original question, this better though. 1+ – Eugene Oct 11 '18 at 12:23
  • @Eugene Sorting is not always bad, i.e. if the list has few elements, `O(NlogN)` is absolutely fine IMO. – fps Oct 11 '18 at 12:26
  • 2
    Both your and Eugene answers are good, but I think this is better. Thanks for the very clear explanation – Leviand Oct 11 '18 at 13:00
  • I think we can add an additional upgrade to the first part of this answer: we can change `map` to `peek` , so we don't need to `return myObject;`... what do you think? `.peek(myObject -> myObject.setActive("0"))` – Leviand Oct 11 '18 at 13:13
  • `peek` is meant for debugging purposes only, that's why I haven't used it. It's one of those methods that actually works, but it's not recommended to be used in business logic by the JDK team itself... See [this question and its thorough answers](https://stackoverflow.com/questions/33635717/in-java-streams-is-peek-really-only-for-debugging) for the arguments. – fps Oct 11 '18 at 13:19
  • 1
    Didn't know that, I'm gonna dig into ! – Leviand Oct 11 '18 at 13:20
  • @FedericoPeraltaSchaffner but a `map` with side effect is not better than `peek`; it’s subject to the same limitations and problems. – Holger Oct 11 '18 at 14:13
  • @Holger Even though in this case they could be used interchangeably, `map` and `peek` have totally different semantics. For example, `map` allows to create and return a new instance of `MyObject` with the `active` property filpped (i.e. if `MyObject` were immutable), while `peek` doesn't allow it. The fact that in this case, the same instance is returned by `map` could be seen as a coincidence. – fps Oct 11 '18 at 14:45
  • `map` *could* return a new instance, but here, it manipulates the source object. Further, it is abused as an action, supposed to be applied on all elements, whereas, there is no guaranty that it will be evaluated on all elements, as that depends on the actual terminal operation and implementation details of the Stream API. E.g. it might be skipped entirely, when performing `count()` on Java 9 or newer. The same problems, you’ll run into with `peek` as well. Likewise, there is no guaranty on execution order or threads, just as with `peek`, though that won’t matter here (just as with `peek`)… – Holger Oct 11 '18 at 17:28
  • @Holger Agreed. That's why I'm telling at the end that the source cannot report itself as being sorted, as the max of a sorted source would be one of its ends, and this might lead to `map` being skipped. The best answer is [HadiJ's](https://stackoverflow.com/a/52758744/1876620), then (which I have upvoted). – fps Oct 11 '18 at 17:39
2

It seems you first need to sort in reverse order, so that the element you are interested in, becomes the first one:

myList.stream()
      .sorted(Comparator.comparing(MyObject::getDate, Comparator.reverseOrder()))
      .skip(1)
      .forEach(x -> x.setActive("0"))

and then skip it, setting the other ones to 0.

Eugene
  • 117,005
  • 15
  • 201
  • 306
  • 4
    The higher costs of sorting, compared to a `max` operation likely outweigh the costs calling `setActive` exactly one more time, like with `myList.forEach(x -> x.setActive("0")); myList.stream() .max(Comparator.comparing(MyObject::getDate)) .ifPresent(x -> x.setActive("1"));` – Holger Oct 11 '18 at 14:18
1

No need to sort.just find max(recent) object by comparing date property.

MyObject recent = Collections.max(list,Comparator.comparing(MyObject::getDate));
list.stream().filter(myObject -> !myObject.equals(recent))
            .forEach(myObject -> myObject.setActive("0"));
Hadi J
  • 16,989
  • 4
  • 36
  • 62