Request for review: std.net.isemail

[Jacob Carlborg]

On 2011-03-22 23:21, dsimcha wrote:
> On 3/22/2011 6:04 PM, Jacob Carlborg wrote:
>> I've now finished the port of Dominic Sayers' PHP is_email function
>> (http://www.dominicsayers.com/isemail) and sending it for review.
>>
>> A few comments:
>>
>> * Due to limitations in std.regex some unit tests fail and are out
>> commented
>>
>> * Due to some bugs (4673, 5744) in Phobos this module contains private
>> functions with fixes for these bugs
>>
>> * The DNS check is not implemented resulting in a few out commented unit
>> tests
>>
>> Github: https://github.com/jacob-carlborg/phobos/tree/isemail
>> Documentation: http://dl.dropbox.com/u/18386187/isemail.html
>>
>
> I haven't reviewed the implementation and don't plan to because I don't
> know enough about what issues might arise with something like this to
> provide good feedback.
>
> My major comment about the API is that helper functions like
> firstChar(), max(), substr(), compareFirstN(), grep() and get() should
> be made private and not included in documentation. If you think they
> might be generally useful, they can be made public and documented once
> we figure out where they belong. They definitely don't belong in the API
> of a module for email address validation, though.

The helper functions are private, I just didn't think of that they would 
show up in the documentation.

> Also a few questions regarding status codes: It looks like the status
> codes are not mutually exclusive.
>
> 1. How are multiple errors handled? Are the status codes or'd? Is one
> returned arbitrarily?

I think the original PHP function returned an array of status codes. 
Hmm, I wonder what happened to this. The D version just returns the 
largest status code.

> 2. Does the multiple status codes issue matter for real use cases?

I have no idea. I think most of the users just want to know if the email 
address is valid or not.

> 3. What are EmailStatusCode.On and EmailStatusCode.Off?

I had some problem figuring out how I wanted to solve this. In the PHP 
version the function takes a parameter, errorLevel, indicating what 
error level you want. Any status code above will be returned as-is and 
status codes below will be returned as "valid". In addition to this you 
can pass "true" or "false". If "true", any status code except "valid" 
will be considered invalid.  If "false", the function just returns 
"true" or "false" rather than an integer error or warning.

The problem I had was that since D is statically typed you cannot mix 
integer and boolean values in one parameter, specially not if the 
integer parameter is an enum. Since I wanted to be able to pass both a 
EmailStatusCode value as errorLevel and "true" and "false" I added 
EmailStatusCode.On and EmailStatusCode.Off representing "true" and 
"false". I should document this.

> Other than that, it looks pretty good to me. It solves a simple (at the
> conceptual level if not the implementation level) problem with a simple
> API.

Thanks.

-- 
/Jacob Carlborg