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

String isNilOrEmpty #37

Closed
knothed opened this issue Sep 1, 2019 · 19 comments
Closed

String isNilOrEmpty #37

knothed opened this issue Sep 1, 2019 · 19 comments

Comments

@knothed
Copy link
Contributor

knothed commented Sep 1, 2019

Sometimes, when dealing with viewModels with contain optional values, I find myself writing code like this:

if (viewModel.text ?? "").isEmpty { ... }

An equivalent alternative is this:

if viewModel.text?.count > 0 { ... }

Still, I think it is not as concise as it should be.

C# provides a method String.IsNullOrEmpty. What do you think about a respective Swift method? E.g. an extension to Optional where Wrapped == StringProtocol providing a property isNilOrEmpty?

Also, because HandySwift already provides an isBlank property, this could be extended to also provide a isNilOrBlank property, if this is a common use-case.

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

Is there a related discussion on the Swift forums? What do they think about such a thing?

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Good question, I will take a look later today.

I just came over the idea to introduce this extension not only to String(Protocol), but to all Collections – it may be equally useful for Arrays, Dictionarys etc.

On the other hand, StringProtocol is no Collection, so there would have to be an additional extension for StringProtocol (or for Substring).

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

Yeah, if implemented at all, it should be implemented at the level where isEmpty is defined.

Also note that I got inspired for the isBlank method by Ruby, which has a blank? method. There's also a present? method in Ruby which does exactly what you're asking for. So we may want to consider something like isPresent for naming then which I think reads more naturally than isNilOrEmpty where you have an or and thus have to think a little more until you understand it. isPresent is clear and concise and has known vocabulary for Ruby developers (and maybe even others?).

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

You're right, isPresent reads more fluently, but I don't think it's as clear, especially for non-Ruby users. My first intention would be that isPresent states if there is a non-nil value wrapped in an optional. Thus it could be easily misunderstood.

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

But that's no misunderstanding, if it's true, that actually means that it's non-nil. Only in the Collection case it additionally means it's not empty, since something is present. 😁

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

But it solves the same issue as your suggestion plus it needs no negation in guard statements.

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

True, you're right :D isPresent is good. I think hasElement would also sound nice, but this would more naturally be a property of Collection, not of Optional<Collection>, so maybe isPresent is best...

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Well, maybe hasElement for Collection is not that bad, because it would allow code like this:

guard viewModel.text?.hasElement else { ... }

It is just a plain negation of isEmpty, but would work perfectly with optional chaining. :)

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

Since isPresent can be applied to any Optional, I think it would be more useful as a name since the feature would be more useful in general.

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

How would you define isPresent if you would want to extend it to all Optionals?
It would definitely collide with the original intent (that collection.isPresent = non-nil and non-empty)

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

Why? On collections we would simply override it. 😉

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Hmm... it would be a bit confusing if it is a general non-nil operator, but behaves differently only on Collections 😁
The user would have to be explicitly informed about this.

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

It's not a non-nil operator. It's an operator, that informs you about presence. For Optional(Int) presence is the existence of an Int value. For a collection "presence" means that it contains elements.

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Well, I would argue that even an empty collection is present in the sense that it exists, in contrast to it being nil.

I think there is a potential for misunderstandings...

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Is there a design rationale on why Ruby called it present? 😁

@Jeehut
Copy link
Member

Jeehut commented Sep 1, 2019

There sure had been. No idea if it can be found anywhere. Please also note that we could first introduce isBlank for all collections instead of only for Strings. That would replace the isNilOrEmpty with a naming we already introduced. Then, we can think of introducing the negative form via isPresent, too.

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

Yeah, sounds good. But this doesn't solve the problem yet, as collection?.isEmpty would actually return false for a Optional<Collection> which is nil.

Therefore, I think it does not suffice to just extend isEmpty to Collections, but it should also be added to Optional<Collection>, returning true when either the optional is .none or .some([]), where [] denotes the empty collection.

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

This would allow me, coming back to the original example, to write:

if viewModel.text.isEmpty ...

, which would eliminate the chaining with "".

@knothed
Copy link
Contributor Author

knothed commented Sep 1, 2019

(Or isBlank, whatever fits better.)

@knothed knothed closed this as completed Feb 10, 2020
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

No branches or pull requests

2 participants