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

Dates #20

Closed
robertmryan opened this issue Feb 3, 2015 · 7 comments
Closed

Dates #20

robertmryan opened this issue Feb 3, 2015 · 7 comments

Comments

@robertmryan
Copy link

I notice that you're writing dates using NSDateFormatter. I'd really encourage you to specify a timezone for your formatter in case the timezone of the device changes at any point. As you've implemented the date logic, dates are stored in the device's then local time zone which can be problematic.

I'd suggest

  • using a date formatter that specifies time zone (e.g. store date strings as Zulu/GMT/UTC, NSTimeZone(forSecondsFromGMT: 0) and include the timezone in the date string, e.g. Z; and
  • locale (e.g. the one used for RFC 3339/ISO8601 dates is NSLocale(localeIdentifier: "en_US_POSIX"); see Q&A 1480).

You might even want to follow one of the established standards, such as ISO8601 format, e.g. 2015-02-03T15:29:00.000Z, in which the format is sortable and unambiguous.

@robertmryan
Copy link
Author

By the way, while this is a somewhat minor issue (though, IMHO, still important) when dealing with data that the end user writes to the database themselves, it's a huge deal when dealing with a database that the app developer includes in a database that they distribute with the app (in which the timezone and locale is that of the app developer, which is, more than likely, completely different from the users').

@robertmryan
Copy link
Author

@kschembri No, I hadn't planned on investing any more time in this. Frankly, my original point was actually that the failure of classes like this to handle dates properly was an indication that they weren't terribly mature. The date formatter was my litmus test. Frankly, the fact that the author of this class hasn't responded to this issue over a month later seems to only reinforce my concern that this class isn't ready for prime time.

Having said that, the fix for this particular issue is trivial: Just set the timeZone and locale properties, as I've outlined above, wherever this class uses a NSDateFormatter. Frankly, the date format string should probably include a Z, too (so the fact that it's Zulu/UTC/GMT is self-evident).

If you issue a pull request, let me know and I'd be happy to take a quick look if you want a second set of eyes on it.

@FahimF
Copy link
Owner

FahimF commented Mar 28, 2015

@robertmryan The reason I did not respond to your point was because the library does not write any date information to the DB using date formatters. So it looked as if you hadn't actually looked at the code and what it did.

I did want to evaluate all your points though and since I've been travelling for a month or so now, have not had the chance to do so.

Rather than criticizing code for not being mature, it would be more constructive if you would submit a pull requrest for any changes that you think are needed. That way, I can actually evaluate if that works rather than try to modify something based on a comment which could possibly be based on an incorrect asumption :)

I wrote the code to meet my own requirements. And I'm happy to share the code in case somebody else finds it useful. But to have critiques about whether the code passes your litmus test or not is really not relevant to me. I'm happy to fix things if there are issues but since the code is freely available, you can also fix the issues if they are relevant to you. That might be more constructive :)

@robertmryan
Copy link
Author

I made three very specific, constructive suggestions of what you needed to do. If I just wanted to criticize, I would have merely pointed out that his class doesn't handle dates well/completely/correctly, and left it at that.

But, no, I'm not going to invest any time fixing your code. I described what you needed to do, but I really don't care if you fix it or not.

@FahimF
Copy link
Owner

FahimF commented Mar 28, 2015

@robertmryan If you don't care if the code is fixed or not, then you could have left things as is - or stopped with the condescension about how things don't pass your litmus test. As I mentioned, the code works fine for my own needs and don't really need anybody fixing things for me. What I was saying was that if you needed it fixed, you might want to do it yourself and submit a pull request.

It's rather annoying that the mere fact of sharing code with others to help them results in people asking you to help them with all their own code issues. To clarify, I'm speaking generally here and not of your specific case. But I do get lots of requests for code changes, help with code, and even help with SQL queries. Don't really have the time to attend to all these things.

@FahimF FahimF closed this as completed Mar 28, 2015
@robertmryan
Copy link
Author

Understood.

Not to drag this out, but let me say that I am really sorry that I offended you.

But I must confess that I'm equally frustrated: You offered up this class in a Stack Overflow discussion about how to use SQLite from Swift. I was merely trying to be constructive in not only pointing out these subtle timezone and locale issues but also outlining precisely how to fix it. But rather than a simple "thanks", I get nothing but radio silence for nearly two months, and then the reply is, effectively, "I didn't respond because I thought you were too stupid to read the code" and "I'm too busy to fix my own code, so you do it". But I get it: The code works for you, so the timezone and locale issues are not relevant in your case. That's fine.

You clearly took great offense to my comment about the NSDateFormatter implementation being indicative that this class was not "terribly mature". (In my defense, I likely would not have proffered this opinion if either you responded to the original issue or to kschembri's follow-up question.) I was a little surprised by the vehemence of that reaction given your own admission that this wrapper is "a bit rough around the edges".

But that's irrelevant. Let me say again, and I mean this sincerely, I apologize for offending you, as no offense was intended. My original issue was truly posted in the spirit of constructive criticism and I'm sorry that it seems to have spun out of control here. I shall refrain from making observations/comments on your repositories in the future.

@FahimF
Copy link
Owner

FahimF commented Mar 28, 2015

@robertmryan No worries and no harm done :) I did see the issue you posted and thought the NSDateFormatter stuff was relevant - just that it didn't write to the database as you mentioned, that confused me originally since I couldn't understand where I wrote to the DB usind NSDateFormatter. So I set it aside to look at later but have since been travelling and things have been crazy.

I finally got a chance to look at this yesterday and then I saw the whole conversation here and I must admit that the latter part of the conversation was what set me off. It's not your fault. This thread had somebody asking that an issue be fixed for them immediately (again, not you) and that's the type of behaviour surrounding releasing open source software that annoys me - the sense of entitlement without any actual contribution.

Personally, I release the source because I think it might help somebody else as is and I'm happy to fix bugs when I can and to incorporate suggested features and fixes when I can. But when people start asking for help with no consideration for my time or demand fixes (again not you), it just annoys me and I think that just spilled over. My apologies if it came over as too harsh/confrontational.

Your issues are relevant and I still intend to look into them. But not just immediately since I just got back from another trip and need a bit of time to unwind :) But thank you for pointing out the issues and do feel free to point out further issues - it really wasn't the original comment that set this whole unfortunate detour in motion ...

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

No branches or pull requests

2 participants