Skip to content

Conversation

ashleyfrieze
Copy link
Contributor

  • Surprising behaviour with Scanner suggests home-made code acted unusually

- Surprising behaviour with Scanner suggests home-made code acted unusually
.execute(() -> {
Scanner scanner = new Scanner(System.in);
scanner.nextLine();
assertEquals("first line", scanner.nextLine());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increase clarity of the second assert

).execute(() -> {
Scanner firstScanner = new Scanner(in);
firstScanner.nextLine();
Scanner secondScanner = new Scanner(in);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first scanner may cache more than just the first line, so the second scanner is unable to pick up the second line.

.execute(() -> {
int numBytesRead = System.in.read(DUMMY_ARRAY, VALID_OFFSET, 0);
assertThat(numBytesRead).isZero();
assertThat(numBytesRead).isEqualTo(-1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read should return -1 at the end of the stream.

- Surprising behaviour with Scanner suggests home-made code acted unusually
- Interface for `read` was broken - should return `-1` at stream end
@ashleyfrieze
Copy link
Contributor Author

I've encountered some surprises with this implementation. I think the home made stream was there to solve a problem that was not covered by the original unit tests for SystemLambda. I've put the missing tests into #18

I'd like a little clarification, please. What's the intended behaviour?

Some of the tests in #18 will likely fail if we use more conventional implementations of InputStream as Scanner tries to read ahead.

@ashleyfrieze
Copy link
Contributor Author

Further review suggests that this is not a viable solution... worth exploring whether the behaviour in #18 is what's expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant