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

Use String/Nil#presence in a few places throughout stdlib #8508

Merged
merged 2 commits into from Nov 28, 2019

Conversation

@Sija
Copy link
Contributor

Sija commented Nov 22, 2019

SSIA

@Sija Sija force-pushed the Sija:moar-presence-here-and-there branch from 401e8c3 to c82e4e4 Nov 22, 2019
@Sija Sija force-pushed the Sija:moar-presence-here-and-there branch from c82e4e4 to 0806125 Nov 22, 2019
@Sija Sija force-pushed the Sija:moar-presence-here-and-there branch from 0806125 to a68f650 Nov 22, 2019
@RX14
RX14 approved these changes Nov 28, 2019
@RX14 RX14 added this to the 0.32.0 milestone Nov 28, 2019
@RX14 RX14 added the kind:refactor label Nov 28, 2019
@RX14 RX14 merged commit 0bb3fe9 into crystal-lang:master Nov 28, 2019
5 of 6 checks passed
5 of 6 checks passed
ci/circleci: test_preview_mt Your tests failed on CircleCI
Details
ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32_std Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asterite

This comment has been minimized.

Copy link
Member

asterite commented Nov 28, 2019

Just note that a && !a.empty? is different than a.presence because the latter might be a bit slower because it needs to traverse all chars, while empty? just checks for bytesize.

I'm not saying that this PR is not good, but with it merged it's not exactly the same code, and it might actually be slightly slower (maybe not).

@Sija

This comment has been minimized.

Copy link
Contributor Author

Sija commented Nov 28, 2019

Just note that a && !a.empty? is different than a.presence because the latter might be a bit slower because it needs to traverse all chars, while empty? just checks for bytesize.

@asterite Good point! Also worth noting is the cause of that - i.e. using String#blank?, instead of String#empty? - therefore every case should be thought out in regards to whether whitespaces (and speed) are sth you care about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.