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

Remove Nulls Deeply #747

Merged
merged 2 commits into from
Jul 28, 2017
Merged

Remove Nulls Deeply #747

merged 2 commits into from
Jul 28, 2017

Conversation

joeydong-stripe
Copy link
Contributor

@joeydong-stripe joeydong-stripe commented Jul 24, 2017

Summary

Improve the NSDictionary / NSArray remove nulls helpers to scan deeply instead of only at the first level.

Motivation

Nulls are particularly difficult to work with in any situation so just filter them out everywhere.

Testing

Updated unit tests.

@bg-stripe
Copy link
Contributor

hmm. my guess is there probably aren't many users at the intersection of

  • use allResponseFields
  • explicitly check for NSNull in a nested dictionary, and don't also check for nil

seems like a worthwhile improvement to me, and worth the small risk of a subtle break as long as we call it out in MIGRATING

@joeydong-stripe
Copy link
Contributor Author

I guess the trickiest thing you can end up doing is something like:

if (card.allResponseFields[@"metadata"][@"username"] != nil) {
    // Do something without actually checking the value of metadata.username
}

@joeydong-stripe joeydong-stripe force-pushed the joey-remove-nulls-deeply branch from 7534652 to 3cccd8d Compare July 25, 2017 20:38
We're going to add a few more helpers to this category but not so many that a separate category namespace is neccessary
Nulls are particularly difficult to work with in any situation so filter them out everywhere
@joeydong-stripe joeydong-stripe force-pushed the joey-remove-nulls-deeply branch from 3cccd8d to a6bbd6e Compare July 28, 2017 19:04
@joeydong-stripe joeydong-stripe changed the title [WIP] Remove Nulls Deeply Remove Nulls Deeply Jul 28, 2017
@joeydong-stripe
Copy link
Contributor Author

r? @bg-stripe

Copy link
Contributor

@bg-stripe bg-stripe left a comment

Choose a reason for hiding this comment

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

lgtm!

@joeydong-stripe joeydong-stripe merged commit 52f7623 into master Jul 28, 2017
@joeydong-stripe
Copy link
Contributor Author

Oops forgot too add an entry to the CHANGELOG

@joeydong-stripe joeydong-stripe deleted the joey-remove-nulls-deeply branch July 28, 2017 22:24
porter-stripe pushed a commit that referenced this pull request Feb 14, 2022
…747)

* Pass new card cvc to Link intent completion

* Include cvc updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants