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

Shouldn't you use an optional instead of empty array? #118

Open
drumnkyle opened this issue Nov 16, 2019 · 5 comments
Open

Shouldn't you use an optional instead of empty array? #118

drumnkyle opened this issue Nov 16, 2019 · 5 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@drumnkyle
Copy link

In the Cascading UI updates including a network request section, you mention making an array even though you expect one value so that you can have an empty one when there is no value. Why wouldn't you use an optional in this case and then compactMap instead? If there is a reason, then I would mention it here.

This is a fantastic resource btw. Thank you very much!

@drumnkyle
Copy link
Author

drumnkyle commented Nov 16, 2019

I read in the code comments that you started to have some kind of explanation about why you didn't use an optional, but it didn't make sense to me. I read more in the comments below and it was a little more clear, but I still don't understand why the empty array helps for removing things. Is it possible to show how the optional version works and why it was problematic?

@heckj heckj added the enhancement New feature or request label Nov 16, 2019
@heckj heckj self-assigned this Nov 16, 2019
@heckj heckj added this to the first edition milestone Nov 16, 2019
@heckj
Copy link
Owner

heckj commented Nov 16, 2019

thanks @drumnkyle - let me see if i can reword that a bit.

the heart of the reason is that optional values (when nil) don’t (or didn’t in my experiment) trigger some of the operators to pass through the pipeline, and in my specific example i wanted to know when the result was “nil” to clear the representing UI element.

@drumnkyle
Copy link
Author

Ohh I see. That explains it. Did you try maybe utilizing the replaceIfNil so that you knew when it was nil? Idk if that would work well because then you’d have to have another value I guess.

@heckj
Copy link
Owner

heckj commented Nov 17, 2019

I didn't, although I suspect your idea is functional and that I could have - at the time, it seemed just as easy to use an array, with the idea that an empty array meant empty value - rather than cobbling some specific value type or class that could encapsulate the idea that "no response is a valid response" so I could clear up the UI displayed. I certainly don't need a full array (only one element for that use case), but I figured "no-code" was good code ;-)

@drumnkyle
Copy link
Author

Hmm. That’s true it is less code. Just seems like more clever and brings about questions for someone new coming to it. Definitely an interesting case. Thanks for going back and forth on it. Maybe I’ll mess around with it at some point. I’m still very new to Combine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants