Is intersect of 2 cohorts handled correctly?

Development
,
1

Hi. I started to work on https://issues.openmrs.org/browse/TRUNK-5331?filter=-1 . To solve the ticket i need to adjust Cohort.intersect() but i noticed that if one of cohort is null the return is a new empty cohort . Is this meant to be like this? code is here https://pastebin.com/qf0LQ75a

2

Mmm, that’s the intersect I knew:

public static Cohort intersect(Cohort cohortA, Cohort cohortB) {
  Set<Integer> ids = new HashSet<Integer>(cohortA.getMemberIds());
  ids.retainAll(cohortB.getMemberIds());
  return new Cohort(ids);
}

@mseaton do you know why this was duplicated in Core? (here thus).

3

@mksd is this what you are looking for? https://issues.openmrs.org/browse/REPORT-829

1 Like
4

@andu033 i do not see a difference between what is in your pastebin link and this https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/Cohort.java#L283-L293

5

Yes indeed! Thanks.

6

Yes. But i wanted to know if the implementation of this is correct. From my point of view if one cohort is empty the instructions of if will not execute so the result cohort will be empty (basiclly new cohort()).

7

And your link is pointing to the lines of subtract method not intersect .

8

Returns the intersection of two cohorts, treating null as an empty cohort . This is in the comments above intersect method . If b is empty we should return a, right? But the method doesn’t do that.

9

@andu033 the best way to move this forward would be to put together a (failing) unit test on your fork that displays the faulty intersect behaviour. And point us to that fork/branch on GitHub.

10

Ok.I will do that. Thanks :slight_smile:

11

Nvm . I mistaken i was thinking about union not intersect . My bad. Sorry