search

billymeltdown / nsdate-helper

A category to extend Cocoa's NSDate class with some convenience functions.
http://www.zetetic.net/blog/
456 stars 107 forks source link

Adding basic i18n support #15

Closed hectr closed 10 years ago

billymeltdown commented 10 years ago

This is excellent, @hectr, thanks very much! Sorry for the delay, but I've merged your pull request into the develop branch as of SHA: d723b62e6846d486ae41d6c7ca8767e5333ecc54 Want to give it a shot? NSDate+Helper has been updated with major performance improvements.

hectr commented 10 years ago

Hi @billymeltdown, today I've been rebasing #14. I believe the string method is mostly useless if its result depends on the user locale. But anyway, I don't think #14 can be merged right now, because it's not using sharedDateFormatter. I can't actually understand how the _displayFormatter's date format could be changed after its initialisation without running into concurrency issues in multi-threaded environments. Could you please share your thoughts about that? Does it mean that NSDate+Helper is not thread-safe anymore?

billymeltdown commented 10 years ago

To be honest I haven't thought real hard about thread concurrency with these helpers methods, although I'm not running into any issues. If you'd like to point out what's problematic with the current setup and suggest some fixes that would be great!

I believe the string method is mostly useless if its result depends on the user locale.

Could you expand on that some more?

All in all, this is a krufty category. I recently came across Erica Sadun's NSDate-Utilities category and it looks quite cleanly implemented, using a shared calendar but not a shared formatter:

https://github.com/erica/NSDate-Extensions/blob/master/NSDate%2BUtilities.m

billymeltdown commented 10 years ago

Did a little reading and you are right, NSDateFormatter has not been thread-safe. However, it is in the very latest APIs (iOS 7 and OS X 10.9 Mavericks):

https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSDateFormatter_Class/Reference/Reference.html

So we'll want to fix this to be thread-safe for older deployment targets, as not everyone is targeting only the latest and greatest (myself included). If you have the time and you want to take a crack at some of it, it'd be a big help! I'll try to bang this out in the near future.

hectr commented 10 years ago

I would gladly contribute to fix it, but I'm afraid I don't fully understand yet the implications of mutating a shared date formatter.

I would probably suggest to use a different shared formatter for each kNSDateHelperFormat*, but perhaps that could ruin the benefits of having a single formatter in most scenarios. Implementing some sort of synchronization mechanism seems an even worse solution for me.

I also thought about having a pool of formatters, but this solution seemed a bit over engineered.

I am sure there must be a smarter way for dealing with the thread safety issue, but it's simply beyond my knowledge.

billymeltdown commented 10 years ago

Well I think the first thing to do is get rid of the shared formatter since that's a no-go before 10.9 and iOS 7, and replace each with individual NSDateFormatter objects to start with. From there we can consider what needs to be optimized, what can be shared because it's never intended to be mutated (e.g. if the user is asking for a format, we need a new formatter, otherwise maybe not).

I remember when this was first suggested, sharing the formatter, it was because it was getting too expensive to use the category due to all the formatter objects created. I suspect that maybe we just shouldn't be doing this and the application code should be handling, but maybe we can thread the needle.