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

Non-optional values #19

Merged
merged 3 commits into from Jun 29, 2015
Merged

Non-optional values #19

merged 3 commits into from Jun 29, 2015

Conversation

@kohtenko
Copy link
Contributor

kohtenko commented Jun 22, 2015

The idea taken from SwiftyJSON. It's really useful in cases, when you don't want care about optionals.

@radex
Copy link
Collaborator

radex commented Jun 22, 2015

That does look useful. Not sure about NSDate() since it represents the current, not "empty" date, but 0, [], [:], "", etc. often do make sense.

@kohtenko
Copy link
Contributor Author

kohtenko commented Jun 22, 2015

I agree, made this value 0 in unixtime on 3b9c29b

@radex
Copy link
Collaborator

radex commented Jun 22, 2015

Still don't know about this… Is 1970 a natural, obvious "null value" that we want people to use, implicitly?

@kohtenko
Copy link
Contributor Author

kohtenko commented Jun 22, 2015

I believe this value is "default enough" :) If someone wants to make sure about null value - there is date and hasKey methods.
unixtime 0 always been debatable value, but everyone knows that this is 01.01.1970

@radex
Copy link
Collaborator

radex commented Jun 22, 2015

Hm, I'll think about this.

Not merging this PR just yet, but I will ;)

@radex radex merged commit 3b9c29b into sunshinejr:master Jun 29, 2015
@radex
Copy link
Collaborator

radex commented Jun 29, 2015

Merged. Thanks.

I removed objectValue and dateValue before merging. I'm not convinced of the usefulness of their default value behavior. I'd rather people do this explicitly.

(If people complain about this and make a convincing argument for it, I'll put it back.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.