Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Allow NSManagedObject formatter to be changed #46

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vnavarro
Copy link

  • changed defaultFormatter to formatter
  • added a sharedFormatter getter
  • exposed setFormatter as public
  • added two tests to verify date formatting when
    using setFormatter

Suggestion: Maybe is also a good idea to add some way of changing each date formatting while mapping.

* changed defaultFormatter to formatter
* added a sharedFormatter getter
* exposed setFormatter as public
* added two tests to verify date formatting when
using setFormatter
+ (void)setFormatter:(NSString *)newFormatter {
[sharedFormatter setDateFormat:newFormatter];
}

Choose a reason for hiding this comment

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

what about name convenience?

- (void)setDateFormat:(NSString *)dateFormat {
    [sharedFormatter setDateFormat:dateFormat];
}

* included podfile.lock
* fixed call for setDateFormat when there is no 
sharedFormatter instance yet
@vnavarro
Copy link
Author

Thanks @viking2009 I think it's much better now.

@supermarin
Copy link
Owner

@vnavarro thanks for your work!

I do have a few concerns before merging this tho:

@stephencelis
Copy link
Collaborator

Format should be changeable per entity, so Person +dateFormat and Message +dateFormat could be different.

Hm, should it be by entity? I think a global setting actually works well here (as most APIs will use a global date format). For granularity we can fall back to the transformation support defined in #66.

@supermarin
Copy link
Owner

I've thought @kattrali said a certain API returns different date formats on a few places

@stephencelis
Copy link
Collaborator

@supermarin In those cases, I think a transformation could solve the problem. An exception instead of a rule.

@kattrali
Copy link
Contributor

@supermarin @stephencelis Agreed that transformation could handle those special cases. It seems reasonable to assume that different date formats per model would be the exception in most API synchronization flows.

@supermarin
Copy link
Owner

Sounds good. The last one - should we just expose the whole readonly -dateFormatter so people can change stuff like timezone?

@stephencelis
Copy link
Collaborator

@supermarin Hm, so maybe the date formatter should go directly on the CoreDataManager class?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants