Review of std.net.isemail part 2

[Jonathan M Davis]

On 2011-04-03 13:13, Jacob Carlborg wrote:
> On 2011-04-03 20:09, Andrei Alexandrescu wrote:
> > On 3/29/11 4:06 PM, Jacob Carlborg wrote:
> >> I've made a few minor changes:
> >> 
> >> * Renamed EmailStatusCode.Off -> None and On -> Any
> >> * Added and clarified the documentation for EmailStatusCode.Any and None
> >> * Updated the documentation
> >> 
> >> Github: https://github.com/jacob-carlborg/phobos/tree/isemail
> >> Docs: http://dl.dropbox.com/u/18386187/isemail.html
> > 
> > Overall I think the module is in good shape for admission, with the nits
> > below. Here is my review:
> > 
> > * First doc sentence: "To validate..." is grammatically mistaken
> > (doesn't have a predicate). Normally I'd just note that and fix later,
> > but this is the first sentence so I guess it gets a higher priority.
> 
> I'll fix that.
> 
> > * Copyright: please look up and use the comment convention used
> > elsewhere in Phobos.
> 
> Ok.
> 
> > * Instead of using a bool for checkDns I suggest enum CheckDns { no, yes
> > } which is used in other parts of Phobos.
> 
> Why is this better? This is just like the pointless aliases you want to
> avoid. Why do we even have a bool type in the language if we're going to
> replace it with enums all the time?

Andrei has been pushing for this, because it makes the calls to the function 
easier to read. If it's CheckDns.yes, then it's clear what you wanted, whereas 
if it were true, you'd have to look up whether true meant that it's supposed 
to check or skip checking. In some cases, that's more useful than others, but 
it's already becoming fairly common in Phobos that such explicit enums are 
used rather than bools.

- Jonathan M Davis