71
Assignment inside a condition is ok in this case, as the assignment is surrounded by an extra pair of parentheses – the comparison is obviously != null, there is no chance that we wanted to type line == reader.readLine().
However, a for loop might actually be more elegant here:
for (String line = reader.readLine(); line != null; line = reader.readLine()) {
...
36
It's more idiomatic to use .map on the stream instead of Collectors.mapping:
stringsMaybe.stream()
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toList());
Without introducing a helper method or a custom collector, that's the shortest and clearest way to do this.
Since Java 9, Optional offers a stream method, enabling you to do ....
29
Two things:
I think you have missed the native implementation. Are you intentionally re-inventing the wheel? Streams have an iterator() method.
Note that streams cannot be reversed, so, while you can create an iterator once, from the stream, you cannot create a second iterator.... In other words, you cannot loop more than once through your iterable.
...
22
I see a few, actually, a number of problems in this code. I am afraid this will be something of a scathing review, in part because you mention this is intended to be for a tutorial....
I see a number of call them 'critical' issues. Then also a number of lesser issues. The critical ones first:
Use Case
Why not use an Iterator? Since an Iterator would do ...
21
Why not:
static string Read(istream &stream, uint32_t count)
{
std::string result(count, ' ');
stream.read(&result[0], count);
return result;
}
Though not strictly C++03 compatible that is easily validated. One of the reasons the committee found it easy to add the new constraint in C++11 was that no implementation did not use ...
21
You could increase the abstraction level of the code a little bit with an iterator-like pattern and the same time you could reuse an existing library (with the experience of the authors) for that: Apache Commons IO LineIterator. It would replace the null check to a little bit readable hasNext()/nextLine().
Using an iterator hides an unnecessary detail: the ...
18
You should not need a special class for this. Method references can be cast to functional interfaces. In this particular case, however, remember that the stream returned by Files::lines needs to be closed to prevent leaks:
try (Stream s = Files.lines(...)) {
for (String v : (Iterable<String>) s::iterator) {
...
}
}
15
I have a few comments that are unrelated to the synchronous/asynchronous and/or header-only nature of the code.
parameters
bools
I don't like passing bools as parameters. I really dislike a function like your SimpleSocketStreamBuffer constructor that take multiple bools. You need to do a fair amount of looking to be sure how:
foo x("www.google.com",...
15
Your code is odd in the sense that it is going to a lot of effort to calculate that 103 is 1000.
I understand why you are doing it, but I took the liberty of changing the count() terminating function and replacing it with:
StringBuilder sb = new StringBuilder();
.......
.forEachOrdered(prod -> sb.append(prod.toString()).append("\n");
This way we can ...
14
Your code is not thread-safe. Each of the threads will, in parallel, be accessing both the best, and the bestQuality variables.
Your Lambda is, in essence, modifying external data from the stream, and this is an anti-pattern for streams. It has side-effects.
You should change your code to use the collect mechanism. There are a few ways to do it, but, you ...
13
Instead of wrapping the reader in an iterator, you could also wrap it in an Iterable that then returns the iterator.
It would allow you to write the following
for (String line: linesOf(reader)) {
// ...
}
which makes very clean code.
12
You don't need the Pair. You can get the String with the maximum length by doing
names.stream().max(Comparator.comparingInt(String::length))
12
After reading the problem description and before reading your code, I implemented something that was very similar to what you already have, which means that your approach to the problem is perfectly fine.
Spliterator - of unknown size
Instead of using
Spliterators.spliterator(it, Long.MAX_VALUE, Spliterator.ORDERED)
You should use
Spliterators....
12
Say what you mean and mean what you say
You named your class "Window" when it should really be something more like "Viewport". There are no "Windows" (in the GUI sense) in your code. If you decide to incorporate this into an application that does contain Windows in the GUI sense, this will be hopelessly confusing.
Don't omit essential documentation from ...
11
I agree with Loki's answer: the difference between calling close explicitly, and letting the destructor call close, is that the destructor will implicitly catch (i.e. conceal) any exception thrown by close.
The destructor must do this (not propagate exceptions) because it may be called if/while there is an exception already being thrown; and throwing a 2nd ...
11
I do not have much to add to @rofl's answer, but I do have another way to show to create Cartesian Products while using streams, which I think might be very interesting aswell.
Important note
I have made a mistake while reading the intention of the OP's post. I have confused Cartesian Products with calculating products of the elements of Cartesian Products....
11
StringWriter and XmlWriter are both IDisposable hence their usage should be wrapped in a using statement
The prefixing o of your local variables smells a bit of hungarian notation which generally does not convey a lot of useful information.
The encoding has already been mentioned in the comments. In .NET strings are UTF-16 so you use should that instead.
...
11
You're a Python native, aren't you? I can tell from the """ multiline strings and the four-space indents and really unidiomatic whitespacing and if/then/else/end instead of ternary and if not instead of unless and use of blocks instead of statements and wow this is really unidiomatic ;-;
Each tip assumes that you've already applied the last.
Also, I'm ...
11
Card
The names SUIT and RANK should be Suit and Rank. I'm not sure about your compareTo for two reasons:
It's inconsistent with equals, which is allowed, but should be avoided if possible and really should be documented.
It accepts any object, but should accept Card only
if(o == null || getClass() != o.getClass()) return 0;
This is wrong..., you'...
11
In order to speed this up, you'll need to avoid as many string creation operations as possible, because they are expensive. Especially the split operation is expensive. Not only does this create many new strings, it does this mostly unnecessarily, because you don't need all the substrings. Instead you need to do some low level searching using only positions ...
10
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 ...
10
The code and documentation look fine in general, so this review will focus on minor optimizations on a per-method basis.
Use ThreadLocal.withInitial
You can set your LOCAL_RANDOM with the following:
private static final ThreadLocal<Random> LOCAL_RANDOM = ThreadLocal.withInitial(Random::new);
You need to give a Supplier<Random> as argument, ...
10
I see a number of things that could help you improve your code.
Decompose your program into functions
All of the logic here is in main in one chunk of code. It may be better to decompose this into separate functions, such as separate ones for input, calculation and output.
Declare variables as late as possible
Rather than using the old C-style of ...
10
The main issue with your code is that it operates with side-effects by leveraging the forEach operation. If you were to run your code in parallel, it would be broken and you wouldn't have the expected result. In this case, what you want is to use a mutable reduction approach, that is collect elements into a container.
All propose solutions hardcodes the ...
10
If you are in for raw performance, try to avoid repeating potentially cost-intensive operations.
In this case, you split the lines twice with the same parameter, which repeatedly applies a regular expression under the hood. Instead of
Sring rName1 = line.split("\t")[2];
String rName2 = line2.split("\t")[2];
Stream<String> s1 = Stream.of(line.split("\...
9
This problem space is not a 'textbook' match to the streams concept. There are going to be some rough edges. The most significant thing is that System.out.println is a 'Side Effect' (and also the collect-to-Map) and is an anti-pattern for a Stream because it makes a parallel stream harder to do. You cannot use the parallel features of streams with the ...
9
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 ...
8
First of all, it seems that you've got a using namespace std; somewhere in your code. Don't do that. (No, really).
Here's a function that should meet your needs.
static std::string Read(std::istream &stream, std::string::size_type count)
{
std::string out;
out.reserve(count);
std::copy_n(std::istreambuf_iterator(stream), count, std::...
8
The code you posted contains no error-recovery at all. It's a bit off-topic on this site to ask how to implement a new feature (error-recovery); but I'll try.
It's not clear what your communication protocol looks like. It might be:
A continuous stream of bytes, to be split into packets of 10
10-byte packets, with a measurable delay between packets, no reply ...
8
Your questions imply that you might not be quite aware of how streams or references work in C#.
You pass in a reference to a Stream
You create a StreamWriter which writes to that Stream
This will automatically make changes visible to anyone holding a reference to the same Stream.
Therefor there is no need to try and return the Stream in any way from the ...
Only top voted, non community-wiki answers of a minimum length are eligible
