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

Added String#presence #8345

Merged
merged 11 commits into from Oct 21, 2019
Merged

Conversation

@igor-alexandrov
Copy link
Contributor

igor-alexandrov commented Oct 18, 2019

Added String#presence method that returns self if not blank, otherwise it returns nil.

Based on ActiveSupport#presence: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/object/blank.rb#L45

This can be useful for params parsing:

state   = params[:state]   if params[:state]
country = params[:country] if params[:country]
region  = state || country || "US"

# becomes

region = params[:state].presence || params[:country].presence || "US"

Or for example CSV parsing (to avoid blank strings in DB):

flip_record.buyer_first_name = csv["buyerFirstName"].presence
flip_record.buyer_last_name = csv["buyerLastName"].presence

Thanks!

@straight-shoota

This comment has been minimized.

Copy link
Member

straight-shoota commented Oct 18, 2019

Similar discussions: #7824, #2413, #1299

But it seems there has never been an explicit discussion about only String#presence. And we already have String#blank?, so it might be a nice addition.

@asterite

This comment has been minimized.

Copy link
Member

asterite commented Oct 18, 2019

I agree. Just having presence is good and would be idiomatic.

@Blacksmoke16

This comment has been minimized.

Copy link
Contributor

Blacksmoke16 commented Oct 18, 2019

Is there any example use case that can't be done with []?

region = params[:state]? || params[:country]? || "US"

or

flip_record.buyer_first_name = csv["buyerFirstName"]?

EDIT: NVM the not blank part makes it more useful

@igor-alexandrov

This comment has been minimized.

Copy link
Contributor Author

igor-alexandrov commented Oct 18, 2019

@Blacksmoke16 good point... to be honest, I forgot about []?. Let me think a bit.

@Blacksmoke16

This comment has been minimized.

Copy link
Contributor

Blacksmoke16 commented Oct 18, 2019

@igor-alexandrov Naw the not blank part makes it better since "" isn't falsy, so wouldn't fallback on the other options if its blank.

# ```
# "".presence # => nil
# " ".presence # => nil
# " a ".presence # => " a "

This comment has been minimized.

Copy link
@bcardiff

bcardiff Oct 18, 2019

Member

maybe we should add an example like

"a".try(&.presence) || "default" # => "a"
"".try(&.presence) || "default"  # => "default"
nil.try(&.presence) || "default" # => "default"

That would be a common use case unless Nil#presence is added.

This comment has been minimized.

Copy link
@jhass

jhass Oct 18, 2019

Member

Let's add Nil#presence

This comment has been minimized.

Copy link
@igor-alexandrov

igor-alexandrov Oct 18, 2019

Author Contributor

Let's add Nil#presence

You mean to add String#presence and Nil.presence?

This comment has been minimized.

Copy link
@jhass

jhass Oct 18, 2019

Member

No, Nil#presence so the following compiles:

data = {} of Symbol => String
value = data[:foo]?.presence || "bar"

This comment has been minimized.

Copy link
@igor-alexandrov

igor-alexandrov Oct 18, 2019

Author Contributor

I made a mistake, of course I meant Nil#presence. Now it makes sense, Nil#presence and String#presence will allow to compile the code above.

This comment has been minimized.

Copy link
@straight-shoota

straight-shoota Oct 18, 2019

Member

@asterite I guess it's useful for general input parsing, not only web. Say configuration files or so.

But then... I've never really needed String#presence in Crystal. Which might mean it's not too important to have it?

This comment has been minimized.

Copy link
@igor-alexandrov

igor-alexandrov Oct 18, 2019

Author Contributor

@straight-shoota @asterite I found that I need this method during CSV file parsing, so this is not only a web-needed method.

This comment has been minimized.

Copy link
@ysbaddaden

ysbaddaden Oct 18, 2019

Member

@asterite blank? and presence come from Rails so we think of them as mostly useful for web development, but they're actually nice to have whenever we deal with user input.

A use case, for example, is dealing with legacy databases where many columns are "" or NULL and should be treated the same, #presence is a life savior.

This comment has been minimized.

Copy link
@asterite

asterite Oct 18, 2019

Member

CSV and DB columns are great examples 👍

This comment has been minimized.

Copy link
@bcardiff

bcardiff Oct 18, 2019

Member

I think that as long as we don't put presence in Object then we are slightly better than #7824.

@igor-alexandrov

This comment has been minimized.

Copy link
Contributor Author

igor-alexandrov commented Oct 18, 2019

Updated usage examples. Added Nil#presence.

src/string.cr Outdated Show resolved Hide resolved
src/nil.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
igor-alexandrov and others added 3 commits Oct 18, 2019
Co-Authored-By: Johannes Müller <johannes.mueller@smj-fulda.org>
Co-Authored-By: Johannes Müller <johannes.mueller@smj-fulda.org>
@igor-alexandrov

This comment has been minimized.

Copy link
Contributor Author

igor-alexandrov commented Oct 18, 2019

Updated usage examples for both String and Nil.

Don't forget that it is Friday today! 🍻

src/string.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
src/nil.cr Outdated Show resolved Hide resolved
src/nil.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
igor-alexandrov and others added 4 commits Oct 20, 2019
Co-Authored-By: Brian J. Cardiff <bcardiff@gmail.com>
Co-Authored-By: Johannes Müller <johannes.mueller@smj-fulda.org>
@bcardiff bcardiff merged commit 16abc5d into crystal-lang:master Oct 21, 2019
6 checks passed
6 checks passed
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
ci/circleci: test_preview_mt Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bcardiff

This comment has been minimized.

Copy link
Member

bcardiff commented Oct 21, 2019

Thanks @igor-alexandrov ! Congrats on your first contribution to Crystal 💯 . I will definitely use this soon.

@igor-alexandrov igor-alexandrov deleted the igor-alexandrov:feature/String#presence branch Oct 21, 2019
@igor-alexandrov

This comment has been minimized.

Copy link
Contributor Author

igor-alexandrov commented Oct 21, 2019

Awesome! 🍻

@bew

This comment has been minimized.

Copy link
Contributor

bew commented Oct 22, 2019

Note: there is no spec for Nil#presence is that intentional?

@igor-alexandrov

This comment has been minimized.

Copy link
Contributor Author

igor-alexandrov commented Oct 22, 2019

I will add it a bit later.

@bcardiff bcardiff added this to the 0.32.0 milestone Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.