Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isNext(char) and isNext(Predicate<Character>) methods to ImmutableStringReader and StringReader #98

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ErrorCraft
Copy link

I see myself and other people use reader.canRead() && reader.peek() == c (or !reader.canRead() || reader.peek() != c) all the time. This is very tedious to do, so this adds the isAt(char) method to the ImmutableStringReader interface and StringReader class.

@ghost
Copy link

ghost commented Jul 25, 2021

CLA assistant check
All CLA requirements met.

@Pokechu22
Copy link

I feel like isAt isn't the best name for this (at makes me think of an index, not a character); maybe nextIs or something like that would be better?

@ErrorCraft
Copy link
Author

Ah yeah, maybe something like isNext to be more consistent with the other method names like canRead? (It also reads a bit better imo)

@ErrorCraft
Copy link
Author

Changed the method name to isNext. I was also thinking of adding an isNext method that accepts a Predicate<Character> as a parameter to be able to check for more than one character. For example canRead() && Character.isWhitespace(peek()) in the skipWhitespace method would become isNext(Character::isWhitespace).

@ErrorCraft ErrorCraft changed the title Add isAt(char) method to ImmutableStringReader and StringReader Add isNext(char) method to ImmutableStringReader and StringReader Jul 29, 2021
@ErrorCraft ErrorCraft changed the title Add isNext(char) method to ImmutableStringReader and StringReader Add isNext(char) and isNext(Predicate<Character>) methods to ImmutableStringReader and StringReader Aug 8, 2021
Comment on lines +81 to +89
@Override
public boolean isNext(char c) {
return canRead() && peek() == c;
}

@Override
public boolean isNext(Predicate<Character> predicate) {
return canRead() && predicate.test(peek());
}

Choose a reason for hiding this comment

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

Would it make sense to implement these as default methods of the interface instead?
(Though assuming no or only a few users implement their own ImmutableStringReader it likely makes no difference)


boolean isNext(char c);

boolean isNext(Predicate<Character> predicate);
Copy link

@Marcono1234 Marcono1234 Nov 25, 2021

Choose a reason for hiding this comment

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

What about using IntPredicate here? This would avoid boxing and unboxing. But maybe it does not matter much. And it might cause confusion, making users believe this handles code points (including > Character.MAX_VALUE), so this would require renaming the parameter to for example charPredicate.

@peterix
Copy link
Contributor

peterix commented Oct 26, 2022

Closing and reopening to rerun checks.

@peterix peterix closed this Oct 26, 2022
@peterix peterix reopened this Oct 26, 2022
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.

4 participants