Re: RFC822 Email parser

Quoting Richard Heyes <php@heyes-computing.net>: > Ok, it's done. Probably. It's rather substantial (600 lines(!)), so where > performance is an issue it probably wouldn't be appropriate. There's > probably bugs too. It returns a slightly different structure than the imap > function, but IMO it's better as it takes into account groups and displays > the addresses in a group nested. > > Before I pearify it, any comments or suggestions? From a quick look, looks pretty good - I'm going on the "looks like it works, and we can tweak it if it doesn't" principle. Comments: 1. I'd suggest making it an RFC822:: class, and having a parseAddressList() method inside it. Also, if you could get away with making it a static class, that might be good. Allowing for a future writeAddressList() function would be a good thing, IMHO. 2. While awareness of groups is a good thing, returning a different format is a bit awkward, since it makes this not a drop-in replacement for imap_rfc822_parse_adrlist. Here's my suggestion: an optional flag to parseAddressList() to turn on the group portion of the parsing. That way you can guarantee compatibility by default, but get the groups if you know you're using this implementation. 3. Following that, if it _is_ compatible, how about checking for the imap_* functions, and using them if they're available? That way we provide a only-very-small-overhead wrapper if the c-functions are available, and transparently implement them for everyone even if they're not. To go along with #2, if groups functionality was requested, we'd just use the PHP implementation even if the imap_* functions were around. 4. I've been subtly calling things parseAddressList() instead of parse_adrlist. Studlycaps is PEAR-standard naming (at least, I _think_ we decided that), and I see no reason to save the few characters; use the full parseAddressList in the name of clarity. Nice chunk of work! -chuck -- Charles Hagenbuch, <chuck@horde.org> "If you can't stand the heat, get out of the chicken!" - Baby Blues