Added :format option to the datetime column type

[cognitiveflux]

*User can specify a parsing format to be used when type casting a date string into an NSDate object

[cognitiveflux]

@sxross: The code in DateParser was designed to handle the common cases, and absent something like the Chronic gem, adding a bunch of special case code seemed outside the problem domain. If this is sufficient to solve #121, please close that, otherwise please look at DateParser.parse_date. There may be a way to generalize this while keeping sensible defaults -- perhaps an optional block argument to set the format? Pull requests welcome!

Within the pull request an optional format: parameter can be supplied in the column definition, which is similar to a user specifying a block as a column parameter. The key difference is that in the case of a block the user would be responsible for returning an NSDate object, versus supplying any properly formatted date string and abstracting NSDateFormatter and NSDate. The custom string format allows explicit definition of any format (no guessing).

As Submitted

class Task
  include MotionModel::Model
  include MotionModel::ArrayModelAdapter

  columns :due_date => {:type => :date, :format => "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ"}
end

As Proposed?

class Task
  include MotionModel::Model
  include MotionModel::ArrayModelAdapter

  columns :due_date => {:type => :date, :parser => ->{ ... } } # Block must return NSDate
end

Right now there's no way to specify an alternate format (well, aside from defining a setter for that attribute), so I'm really just asking for suggestions on how to best add that capability to MotionModel. Instead of a column parameter, what about a model parameter, such as:

class Task
  include MotionModel::Model
  include MotionModel::ArrayModelAdapter

  parse_dates_with_format "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ"

  columns :name        => :string,
          :long_name   => :string,
          :due_date    => :date
end

Open to ideas!

[sxross]

I'm not crazy about a macro because it would be global. What if you had two remote sources that supplied (gasp!) two different formats? I think your initial as-submitted is good so long as the user knows to use the Apple formatters. If they don't cross-reference the Apple documentation, the :parser option makes sense because it shifts the burden to them. Their needs may not coincide with what we expect. We can support both, with :parser having higher priority, but the presence of both issuing a warning.

Thoughts?

[cognitiveflux]

I know that that Apple currently conforms to UTS#35, I'm just not familiar with how much the Date Field Symbols vary (e.g. when does 'yyyy' mean something else?). So on that note, I completely agree that it's difficult to know what others will expect or need, so having both makes sense. The parser approach also allows someone to use external libraries for much more complex parsing, which is nice! I don't have an issue dropping the format: option in favor of just the parser: to keep it simple, it's really there as more of a convenience option but it means that there is more responsibility placed on MotionModel for datetime parsing. Based on feedback you've seen from how people are using MotionModel, I'll defer to your recommendation on whether it should be both options or just the parser: option.

From a performance concern, in the case of the parser: it would be up to the user to use a singleton for NSDateFormatter, otherwise it will create new parser for every model instance, but that could just be a documentation note.

I'll have to see what's available within MotionModel to issue the warning, but I agree with your approach.

[sxross]

Good point about the singleton. If people don't understand the implications of date parsing using NSDateFormatter, there could be some memory leaks ... or MotionModel will have to do some pretty defensive programming work. Again, :parser with a stern warning not to create lots of heavyweight objects would probably move the parsing work into the consumer's problem domain, which is (IMO) where it belongs.

My experience with users of MotionModel is that they love-love-love convenience, so whatever gets thrown in there is great by them. That said, I've had enough date handling issues that I feel there is not a right answer in the data management layer. Shall we go with :parser?

[cognitiveflux]

As much as I like the convenience option, when it comes to datetime, I think it can become the column type that keeps adding convenience options and taking development time away from the core of MotionModel, so I agree with you that it should be moved to the consumer's problem domain.

I can include an example in the documentation for doing what this original pull request submitted using the new parser: option.

Would you prefer me to create a new pull request with the parser: implementation or just commit to this branch?

[sxross]

Same branch is fine.