Maybe you are thinking too complicated by trying to find a Stream API solution. The task can be easily solved using basic Collection API operations:
List<Timeslot> timeslots;// the source
TreeSet<Timeslot> sorted=new TreeSet<>(timeslots);
if(sorted.size() != timeslots.size())
throw new IllegalArgumentException("Every timeslot must strictly precede the following");
// get a list where every timeslot strictly precedes the following:
timeslots=new ArrayList<>(sorted);
The equality of the TreeSet’s elements is only determined by the order. So if there are duplicates according to the order, and this is what your problem is all about, they are removed when converting the list to the set. So if the size differs, there were such duplicates. If not, you can convert the sorted set back to a list, if you need a list. Otherwise, you can just continue using the set instead of the list.
It should be mentioned, since you stated to have a ordering which is not consistent with equals, that the TreeSet will also not be fully compliant with the Set contract as stated in its documentation:
Note that the ordering maintained by a set (whether or not an explicit comparator is provided) must be consistent with equals if it is to correctly implement the
Setinterface. […] This is so because theSetinterface is defined in terms of theequalsoperation, but aTreeSetinstance performs all element comparisons using itscompareTo(orcompare) method, so two elements that are deemed equal by this method are, from the standpoint of the set, equal. The behavior of a set is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of theSetinterface.
Since your operation does not rely on the general contract of the Set interface, as there was no Set before, this is not a problem for this specific use case. Converting to a List as shown above works and so would iterating over the elements, in case you use it without conversion, i.e. don’t need specific list operations.
Maybe you are thinking too complicated by trying to find a Stream API solution. The task can be easily solved using basic Collection API operations:
List<Timeslot> timeslots;// the source
TreeSet<Timeslot> sorted=new TreeSet<>(timeslots);
if(sorted.size() != timeslots.size())
throw new IllegalArgumentException("Every timeslot must strictly precede the following");
// get a list where every timeslot strictly precedes the following:
timeslots=new ArrayList<>(sorted);
The equality of the TreeSet’s elements is only determined by the order. So if there are duplicates according to the order, and this is what your problem is all about, they are removed when converting the list to the set. So if the size differs, there were such duplicates. If not, you can convert the sorted set back to a list, if you need a list. Otherwise, you can just continue using the set instead of the list.
It should be mentioned, since you stated to have a ordering which is not consistent with equals, that the TreeSet will also not be fully compliant with the Set contract as stated in its documentation:
Note that the ordering maintained by a set (whether or not an explicit comparator is provided) must be consistent with equals if it is to correctly implement the
Setinterface. […] This is so because theSetinterface is defined in terms of theequalsoperation, but aTreeSetinstance performs all element comparisons using itscompareTo(orcompare) method, so two elements that are deemed equal by this method are, from the standpoint of the set, equal. The behavior of a set is well-defined even if its ordering is inconsistent with equals; it just fails to obey the general contract of theSetinterface.
Since your operation does not rely on the general contract of the Set interface, as there was no Set before, this is not a problem for this specific use case. Converting to a List as shown above works and so would iterating over the elements, in case you use it without conversion, i.e. don’t need specific list operations.
I think sorting the list and comparing adjacent elements is a reasonable approach. Here's the streams-based code that does that:
timeslots.sort(null); // sort the list using natural order
if (IntStream.range(1, timeslots.size())
.map(i -> timeslots.get(i-1).compareTo(timeslots.get(i)))
.anyMatch(i -> i == 0)) {
throw new IllegalArgumentException("...");
}
It's no shorter than what you have, but the streams-based approach might offer more flexibility: it can be run in parallel, you can count the number of duplicate timestamps, etc.
You could also inline the compareTo() call into the anyMatch() call, like so:
if (IntStream.range(1, timeslots.size())
.anyMatch(i -> timeslots.get(i-1).compareTo(timeslots.get(i)) == 0) {
Whether this is preferable is, I think, a matter of style.
static void assertStreamEquals(Stream<?> s1, Stream<?> s2) {
Iterator<?> iter1 = s1.iterator(), iter2 = s2.iterator();
while(iter1.hasNext() && iter2.hasNext())
assertEquals(iter1.next(), iter2.next());
assert !iter1.hasNext() && !iter2.hasNext();
}
Collecting the stream under test (as you show) is a straightforward and effective way of performing the test. You may create the list of expected results in the easiest way available, which might not be collecting a stream.
Alternatively, with most libraries for creating mock collaborators, one could mock a Consumer that "expects" a series of accept() calls with particular elements. Consume the Stream with it, and then "verify" that its configured expectations were met.
There cannot be a simpler solution:
- To know the duplicated ids, you must iterate over the entire collection.
- To print all the persons with duplicated ids, you must keep their full list.
As such, you will need to load the entire collection of persons in memory. There's no way around that. If you needed only the duplicate ids but not the Person objects, then you could keep just the ids with their counts, and throw away the Person objects as you go after use, that would be more efficient. (But that's not the case here.)
In any case, your solution can be more concise if you skip the intermediary map variable with the mapping of ids to lists of users:
people.stream()
.collect(Collectors.groupingBy(Person::getId)).values().stream()
.filter(peopleWithSameId -> peopleWithSameId.size() > 1)
.forEach(peopleWithSameId -> System.out.println("People with identical IDs: " + peopleWithSameId));
Btw, in case you're wondering if the .stream() there could be .parallelStream(), it would be pointless, due to the synchronization in the println method of System.out (a PrintStream). (And without synchronization println wouldn't be thread safe anyway.)
Your code and Java 8 usage looks fine in general to me.
I do see an issue with the Person class, it looks like you are intending it to be an immutable class, if so, then you should also enforce it.
You need to ensure that the name and id fields can never be changed, you can do this by adding final to them. Your code currently seems to be safe, but it is not. I can extend Person and offer a method there to change the name and id fields, which violates the assumed variant of that those fields in Person are immutable.
Simply changing it to the following will do:
public class Person {
private final String name;
private final String id;
...
}
Onto the Java 8 usage now.
It is a good thing that you use the Collectors.groupingBy to provide a Map<String, List<Person>>, you cannot do it much faster either way if you want it to work with any kind of List<Person> as input and in this way you'll save yourself from nasty bugs and reimplementing what lots of people have already done, namely the grouping by operation.
Printing the offending values using Stream seems fine as well, except that you may rewrite it to look a little bit cleaner, something like this could work:
peopleById.values().stream()
.filter(personList -> personList.size() > 1)
.forEach(personList -> System.out.println("People with identical IDs: " + personList);
This is my personal preference on how to format it though, the only real change is to rename peopleWithSameId to personList, as it is simply a List<Person> and nothing more or less.
You've done a good job overall.
Using the Stream API (Java 8+)
boolean allEqual = list.stream().distinct().limit(2).count() <= 1
or
boolean allEqual = list.isEmpty() || list.stream().allMatch(list.get(0)::equals);
Using a Set:
boolean allEqual = new HashSet<String>(tempList).size() <= 1;
Using a loop:
boolean allEqual = true;
for (String s : list) {
if(!s.equals(list.get(0)))
allEqual = false;
}
Issues with OP's code
Two issues with your code:
Since you're comparing
Strings you should use!templist.get(i).equals(first)instead of!=.You have
return true;while it should bereturn flag;
Apart from that, your algorithm is sound, but you could get away without the flag by doing:
String first = templist.get(0);
for (int i = 1; i < templist.size(); i++) {
if(!templist.get(i).equals(first))
return false;
}
return true;
Or even
String first = templist.get(0);
for (String s : templist) {
if(!s.equals(first))
return false;
}
return true;
The frequency of a value in a list will be the same as the size of the list.
boolean allEqual = Collections.frequency(templist, list.get(0)) == templist.size()
Your immediate problem is trivial to fix:
Optional<Integer> oi = nums.stream()
.filter(x -> x > nextNumber)
.findFirst();
System.out.println(oi.isPresent()? "Found: "+oi.get() : "Not found");
However, if you want to write code which optimally computes what you require it to, it is not the right approach. A far better option would be this:
OptionalInt oi = Stream.of(scn.nextLine().split(" "))
.mapToInt(Integer::parseInt)
.filter(i -> i > nextNumber)
.min();
System.out.println(oi.isPresent()? "Found: "+oi.getAsInt() : "Not found");
The advantage is that you never get an ArrayList involved and don't need to autobox the integers at any step, and you actually retrieve the smallest number satisfying the criterion, not the first one.
If you want to find the smallest number larger than some bound:
private int smallestLargerThan(int x, List<Integer> list) {
return list.stream().filter(n -> n > x).mapToInt(n -> n).min();
}
.filter(n -> n > x) drops all values less than or equal to x.
.mapToInt(n -> n) transforms the Stream to an IntStream, which is required for the next operation:
.min() returns the smallest element in the IntStream. Since the stream at this point only contains values greater than x, the returned element is the number you are looking for.
The code you posted won't compile because .findFirst() returns an Optional<Integer>, not an Integer. It is also semantically wrong because the first element is not necessarily the smallest as your Stream is unsorted.
By far my favorite is to use the org.apache.commons.io.IOUtils helper class from the Apache Commons IO library:
IOUtils.contentEquals( is1, is2 );
Something like this may do:
private static boolean isEqual(InputStream i1, InputStream i2)
throws IOException {
ReadableByteChannel ch1 = Channels.newChannel(i1);
ReadableByteChannel ch2 = Channels.newChannel(i2);
ByteBuffer buf1 = ByteBuffer.allocateDirect(1024);
ByteBuffer buf2 = ByteBuffer.allocateDirect(1024);
try {
while (true) {
int n1 = ch1.read(buf1);
int n2 = ch2.read(buf2);
if (n1 == -1 || n2 == -1) return n1 == n2;
buf1.flip();
buf2.flip();
for (int i = 0; i < Math.min(n1, n2); i++)
if (buf1.get() != buf2.get())
return false;
buf1.compact();
buf2.compact();
}
} finally {
if (i1 != null) i1.close();
if (i2 != null) i2.close();
}
}