On Tuesday, March 27, 2012 4:26:37 AM UTC-7, Tom Wardrop wrote: > > I just have a couple of suggestions for Sequel that I'd like to get some > community feedback on, hopefully getting a bit of weight behind them so > there's less debate over implementing these changes. > > 1) I believe #length should be added to Sequel::Dataset as an alias of > #count / #size? My rationale here is one of principle of least > surprise. Besides Sequel, I don't know of any other library that has an > objects that implements a length-like method but calls it something > different to every other Ruby library. I've been using Sequel for 6 months > now and still make the mistake of calling #length. While I respect the > argument that this helps force people to distinguish between an Array and a > Dataset, I don't think that ideal takes human nature into account. >
For background, people should look at https://github.com/jeremyevans/sequel/issues/457 and http://comments.gmane.org/gmane.comp.lang.ruby.sequel/1359. I'm still against this for reasons I gave earlier. The main reason is while datasets are enumerable, they are not like arrays. Datasets don't have a defined size/length. dataset.count == dataset.count is not necessarily true. Still, I'm open to adding the alias if the community is strongly in favor. FWIW, Dataset#size was removed in 3.0. > 2) While I like the simplicity of the validation implementation, I'd like > so see an alternate "smarter" interface. The problem I have at the moment > is that for a given field, I may have numerous validation checks. > Typically, if a field fails validation once, you don't want all the > validation that apply to that field to be run. I'll present an example > problem scenario below, and then demonstrate how it could be handled more > gracefully if Sequel had a smarter validation syntax. > > https://gist.github.com/2214994 > > There's a couple of advantages to the new syntax demonstrated in that > example. The first problem with current validation strategy is that if > :place_id is not present, the second validation will continue to run, > raising a method_missing exception as clearly if :place_id is not present, > then the :place association is going to be nil. These circumstances can > sometimes be hard to spot, and may not come pop up in testing for some > time. The second problem is that some validations could be expensive, so > you only want to validate data that hasn't already been deemed invalid. I > may not be checking the parent_id of the associated place, but may instead > be querying an external web service, as you may do if validating an entered > URL or the existence of a remote object. There's also a third advantage, > and that is that the new syntax is more concise. The condition is implied > (no need for "unless" keyword) an a block wraps better than a trailing > condition. You also get a private variable scope which you know what > conflict with any other variable that may be used for other validations. > > The implementation would be really simple. It's a matter of adding a new > method #validates (or even just allowing Sequel::Model::Errors#add to take > a block). This method checks the Errors hash to see if there are any errors > for the current field. If there isn't, the block is run. A return value > that resolves to true is considered a pass, a value that resolves to false > is considered a fail. > > Thoughts on those two suggestions? > Both of the validation syntaxes that Sequel uses are implemented as plugins, the only built-in validation support is using Errors#add. Are you proposing modifying validation_helpers or adding a new plugin? In your case, you appear to want to skip running multiple validations on an field if a previous validation on the field has failed. I'm not sure how common a desire that is. The main purpose of validations is giving nice error messages to the user (data integrity should be enforced with database constraints), and I would think you would want to report all failing validations to the user. The alternative is the user submits a form, gets an error, submits the form again, and gets a different error message. I'm not saying what you want is a bad idea in your case, but I'm not sure it's a good idea in general. Looking at your gist, you want to change: errors.add(:place, "must have a parent") unless place.parent_id to: validates :place, "must have a parent" { place.parent_id } I find the former syntax more descriptive. You can avoid the NoMethodErrorerror by doing: errors.add(:place, "must have a parent") unless place && place.parent_id Note that that validation still doesn't do what you want. It's possible both for place_id and place.parent_id to be present yet not point to an existing object. A better example would probably be: errors.add(:place, "must have a parent") unless place && place.parent I'm not sure how you want your example to work. You don't want multiple validations in the same field, but in your example, you are using two separate fields. So with an implementation like this: def validates(field, message) unless errors.on(field) errors.add(field, message) unless yield end end yielding to the block may still raise an error in your example. Are you proposing to rescue such errors? Or in your example did you mean to do "validates_presence :place"? Personally, I don't see a need for this. It should be fairly easy to monkey patch in and unlikely to break things. However, I'm certainly open to community feedback in this area. Thanks, Jeremy -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To view this discussion on the web visit https://groups.google.com/d/msg/sequel-talk/-/xlzXLlyfEPkJ. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sequel-talk?hl=en.
