Fixing edge case and adding documentation to String to Date/Time/DateTime conversion methods #5432

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

jasondew commented Mar 14, 2012

Specifically, making it where, say, "foo".to_date doesn't cause an exception.

Member

drogus commented Mar 14, 2012

I'm not sure if that's how it should look like, considering that "1".to_time will raise an exception, why should passing other invalid values silently return nil? I know that it returns nil for blank string, but this seems to be kind of different case, cause there is no info there.

Contributor

jasondew commented Mar 14, 2012

What I would like is for all of these functions to return nil if it can't parse a date. I think that's a better solution than throwing an exception. I'll work on fixing the integer string case if we agree on the overall goal.

Member

drogus commented Mar 14, 2012

I doubt it will be accepted. It's debatable (I mean, there is no right way to do it) and it will break backwards compatibility.

Member

drogus commented Mar 21, 2012

I'm closing this as there is no clear win of returning nil over raising exceptions that would justify breaking backwards compatibility.

@jasondew thanks for the patch anyway

drogus closed this Mar 21, 2012

Contributor

jasondew commented Mar 21, 2012

no worries, it was worth a try :)

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