Skip to content

Conversation

@tomaskraina
Copy link

It would be nice to provide more natural way to compare instances of NSDate -- using <, <=, >, and >= operators. This PR implements all methods declared in Comparable protocol for NSDate class.

In order to provide more “Swifty” API for comparing instances of ‘NSDate’, all methods declared in ‘Comparable’ protocol are implemented for ’NSDate’.
@dzenbot
Copy link

dzenbot commented Dec 4, 2015

👏

Copy link

Choose a reason for hiding this comment

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

Hi @tomaskraina. Great, simple and readable solution 👍
I'd like to suggest a small performance optimisation here: if lhs and rhs are equal you actually call the compare method 2 times on the same pair. Once inside the > method and then once again in ==. Instead, you can save the result of compare and use a switch statement to accept the valid cases or even use some pattern matching magic :-)
Since this is the foundation library, IMO we need to be conscious about performance related decisions, even if they're small.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomaskraina, isn't implementing == and < enough to make it conform to Comparable? Anyway, this is the same as pull request #19.

@tomaskraina
Copy link
Author

@lorentey good point! Updated.

@parkera
Copy link
Contributor

parkera commented Dec 5, 2015

This pull request contains an API change, so let’s follow the Swift evolution process, starting with discussing the idea on the swift-evolution and swift-corelibs-dev lists.

@parkera parkera closed this Dec 5, 2015
atrick pushed a commit to atrick/swift-corelibs-foundation that referenced this pull request Jan 12, 2021
[build-system] Various minor refactoring and cleanups
kateinoigakukun pushed a commit to kateinoigakukun/swift-corelibs-foundation that referenced this pull request Apr 29, 2021
[REPL] Test RPATH is set correctly on linux.
kateinoigakukun pushed a commit to kateinoigakukun/swift-corelibs-foundation that referenced this pull request Oct 11, 2023
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

Successfully merging this pull request may close these issues.

5 participants