Consistent return values

[alexdunae]

This has bothered by for a long time: https://github.com/validates-email-format-of/validates_email_format_of/blob/master/lib/validates_email_format_of.rb#L68

The method either returns nil (for success!) or an array with a single element, the message. It was a design mistake long ago that would be wonderful to fix one day. I'm not sure how without breaking compatibility. Maybe a 2.0 down the road. It should either always return a bool, or always return an array.

Anyway, one day...

[alexdunae]

The one option that does make sense to me would be to add ValidatesEmailFormatOf::valid?(email) as a simple wrapper, returning true or false, and promote it as the preferred entry point. We could leavevalidate_email_format` untouched and just use it internally to feed ActiveModel validations (which it was designed to do).

[betesh]

If it only ever returns one message, does it really need to be an array? I think that introduces confusion in that, if I see an array with only one element, I assume that if there were more than one problem, they'd all be listed there. So how about making validates_email_format_of return a String (or nil if it's valid), and valid? / invalid? just checking whether validates_email_format_of returns nil .

[alexdunae]

I definitely see your logic. I like the idea of only returning a single type (e.g. bool, array) rather than having to do a nil check, so if it was to be a string then the other return value would be an empty string. But that's starting to seem strange...

In the interest of compatibility I think it makes sense to just leave the original method as is and add a single valid? method that return true/false. The other one can remain a franken-method without hurting anything

[betesh]

@alexdunae, What do you think of making the valid? method a core ext on Object and String? i.e.

class Object
    def valid_email?(options={})
      false
    end
end

class String
    def valid_email?(options={})
      ValidatesEmailFormatOf::validates_email_format_of(self, options).empty?
    end
end

[alexdunae]

I think it would probably be better as a module method at the gem root. Monkey patching core classes is just asking for trouble.

If people want it you could include that code in the README, but lots of people won't want it.

Dialect // 778-992-2229 // dialect.ca

On Aug 16, 2014, at 9:55 PM, betesh notifications@github.com wrote:

@alexdunae, What do you think of making the valid? method a core ext on Object and String? i.e.

class Object
def valid_email?(options={})
false
end
end

class String
def valid_email?(options={})
ValidatesEmailFormatOf::validates_email_format_of(self, options).empty?
end
end
—
Reply to this email directly or view it on GitHub.