Support ISO8601 dates by specifying “ISO8601” value for the “dateFormat”… #619

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
5 participants

Support ISO8601 dates by specifying “ISO8601” value for the “dateFormat” User Info key. (https://www.evernote.com/shard/s3/sh/2146a5e0-2440-45ab-9dc8-5f3800d7d1b8/a6fa0ff24d77ca1794a0a842b9f71aab)
Depends on ISO8601DateFormatter pod.

Contributor

tonyarnold commented Dec 12, 2013

Thanks for the pull request, @dperetti! Sorry to do this, but this PR needs discussing before it can be merged — I see two issues:

  1. Currently, MagicalRecord has no dependencies on external libraries or code. This is by design, so this would be a pretty big shift for us. I'm not sure it's the right thing to do here (to be clear, this has nothing to do with @boredzo's code — I use ISO8601DateFormatter in a number of my projects);
  2. MagicalRecord does not require the use of CocoaPods. This PR would break the project for a large number of users who do not use CocoaPods.

@magicalpanda/team-magicalrecord what are your thoughts, guys?

Alternatives might be to git subtree ISO8601DateFormatter into a Vendor subdirectory and add a prefix to the method names, but honestly I think this might be better off as a separate "extension" to MagicalRecord — still included in the main project, but not activated by default (this can be done in the podspec as well).

There should be tests for this new code before it's merged as well.

Owner

casademora commented Dec 12, 2013

If there were to be dependencies for magical record, it would be at the git level.
I still think @tonyarnold is correct, though, we need to not have dependencies as much as possible. Would it be such a base thing to have a Vendor folder with the source fully included in the project? I think that is a simple way to reuse the date formatting code and stay away from adding a repo dependency.

Specifcally for this request, we need to not use singletons. Especially those created with dispatch once. The general habit tends to be it’s created, but never destroyed. It’s poor memory management, and laziness. Even though the current way is slower, it helps keep memory usage in check.

On Dec 11, 2013, at 5:49 PM, Tony Arnold notifications@github.com wrote:

Thanks for the pull request, @dperetti! Sorry to do this, but this PR needs discussing before it can be merged — I see two issues:

Currently, MagicalRecord has no dependencies on external libraries or code. This is by design, so this would be a pretty big shift for us. I'm not sure it's the right thing to do here (to be clear, this has nothing to do with @boredzo's code — I use ISO8601DateFormatter in a number of my projects);
MagicalRecord does not require the use of CocoaPods. This PR would break the project for a large number of users who do not use CocoaPods.
@magicalpanda/team-magicalrecord what are your thoughts, guys?

Alternatives might be to git subtree ISO8601DateFormatter into a Vendor subdirectory and add a prefix to the method names, but honestly I think this might be better off as a separate "extension" to MagicalRecord — still included in the main project, but not activated by default (this can be done in the podspec as well).

There should be tests for this new code before it's merged as well.


Reply to this email directly or view it on GitHub.

As for memory usage is concerned, instantiating the same NSDateFormatter over and over, like 1000 times per sec for nothing seems worse to me :-).
Memory usage is not only keeping it low on the average, it's making a wise use of it, all the time.
IMHO, MagicalRecord should even use some "sharedManager" singleton that would hold objects like this formatter, but also the model introspection data which apparently currently rebuilt over and over.
Some [MagicalRecord cleanUp] call could purge this manager when keeping memory low is important.

Owner

casademora commented Dec 12, 2013

I have some ideas to make that all work with memory and performance in mind. I've got a few other things ahead in the queue at the moment. I agree allocing a formatter like that isn't great, but I still avoid singletons as much as possible.

Sent from my iPhone

On Dec 11, 2013, at 6:16 PM, dperetti notifications@github.com wrote:

As for memory usage is concerned, instantiating the same NSDateFormatter over and over, like 1000 times per sec for nothing seems worse to me :-).
Memory usage is not only keeping it low on the average, it's making a wise use of it, all the time.
IMHO, MagicalRecord should even use some "sharedManager" singleton that would hold objects like this formatter, but also the model introspection data which apparently currently rebuilt over and over.
Some [MagicalRecord cleanUp] call could purge this manager when keeping memory low is important.


Reply to this email directly or view it on GitHub.

xslim commented Dec 19, 2013

you can specify it as optional dependency and later check #ifdef COCOAPODS_POD_AVAILABLE_XXX

dperetti commented May 2, 2014

My latest commit dperetti/MagicalRecord@5826242 now provides ISO8601 support when ISO8601DateFormatter is available, as @xslim suggested.

dperetti changed the title from Support ISO8601 dates by specifying “ISO8601” value for the “dateFormat”... to Support ISO8601 dates by specifying “ISO8601” value for the “dateFormat”… May 16, 2014

tonyarnold closed this Oct 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment