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

Fix NSDateFormatter issues #621

Merged
merged 6 commits into from
Oct 21, 2015

Conversation

erysaj
Copy link
Contributor

@erysaj erysaj commented Oct 13, 2015

Attempt to solve crashes in NSDateFormatter still observed in the wild.
Stop using so-called "hybrid approach" when implementation tries to determine in runtime what NSDateFormatter instance to use: the shared one or retrieved from thread dictionary. Now a library user must decide if thread-safe but more expensive behaviour is desired (default) or simple storing date formatter in ivar is sufficient (suitable for using formatter with a single logger).

A few more minor tweaks:

  • make simple customisations (like changing date format) easy by exposing a method to configure used NSDateFormatter instance
  • set date formatter's calendar once instead of doing that each time a log message is formatted
  • use POSIX locale to have consistent timestamp. For example, currently on devices with Arabian regional settings you'll see digits that you've probably never seen before.

Stop using so-called "hybrid approach" when implementation tries to determine in runtime what NSDateFormatter instance to use: the shared one or retrieved from thread dictionary. Now a library user must decide if thread-safe but more expensive behaviour is desired (default) or simple storing date formatter in ivar is sufficient (suitable for using formatter with a single logger).
…ride point to configure used NSDateFormatter.
Class cls = [self class];
SEL configMethodName = @selector(configureDateFormatter:);
Method configMethod = class_getInstanceMethod(cls, configMethodName);
for (;;) {
Copy link
Member

Choose a reason for hiding this comment

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

Probably while (YES) may be better. Nor sure if do can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rewrite using while with non-trivial condition and without break-s

@rivera-ernesto
Copy link
Member

Related to #33, #53, #452, #296, #44, #39.

@bpoplauschi bpoplauschi added this to the 2.1.0 - Swift milestone Oct 15, 2015
@bpoplauschi
Copy link
Member

@erysaj I think your implementation looks strong, so I will go ahead and merge it. We will keep an eye on this.

bpoplauschi added a commit that referenced this pull request Oct 21, 2015
@bpoplauschi bpoplauschi merged commit 2ee2ac7 into CocoaLumberjack:master Oct 21, 2015
@erysaj erysaj deleted the fix-nsdateformatter-issues branch November 3, 2015 18:27
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.

None yet

3 participants