Hi Roger, Thanks for the review.
> On Sep 9, 2019, at 7:35 AM, Roger Riggs <roger.ri...@oracle.com> wrote: > > I would lean toward updating the spec to reflect the current implementation. It seems strange however that if one read an entire stream using read() in one case and readLine() in another that the results would differ. > A simple program that uses readline and prints the line number and line would > show a duplicate line number. > > try (LineNumberReader r = > new LineNumberReader(Files.newBufferedReader(Path.of(args[0])))) { > String s; > while ((s = r.readLine()) !=null) { > System.out.printf("%3d: %s%n", r.getLineNumber(), s); > } > } > > 1: 123 > 1: ABC For a file with this contents (two lines) 123 ABC I see the same result for the above code both with and without my patch: 1: 123 2: ABC > Other questions and comments: > > - Is the use of AtomicBoolean due to concurrency concerns? > If not, a new boolean[1] would be less overhead No concurrency so that would be better. > - BufferedReader also keeps track of CR/LF pairs and I've thought it would > simpler > not to duplicate the line number counting logic in LineNumberReader. > If BufferedReader kept track of the line number, it could expose a package > private field > to LineNumberReader to make it visible to the get/SetLineNumber methods of > LineNumberReader. That could be a good simplification. Thanks, Brian