search

houseabsolute / Courriel

High level email parsing and manipulation
https://metacpan.org/release/Courriel/
Other
6 stars 5 forks source link

Allow caling to() and cc() with zero arguments in Courriel::Builder #8

Closed alranel closed 6 years ago

alranel commented 6 years ago

Currently, to() and cc() require at least one argument, otherwise they'd fail like this:

Got 0 parameters but expected at least 1 for an un-named validation subroutine

Trace begun at (eval 1226) line 8
Eval::Closure::Sandbox_750::__ANON__ at /usr/local/share/perl/5.18.2/Courriel/Builder.pm line 206
Courriel::Builder::cc at foo.pl line 49

However, since the build_email() syntax is not very OOP and does not make conditional building easy, a quick way for adding recipients only when we have them is needed. In case we supply an empty list, I'd just expect to() and cc() to behave as no-ops.

This pull request leverages the optional argument provided by Params::ValidationCompiler.

autarch commented 6 years ago

I don't think this is something I'd want to add. This seems like it could easily lead to a lot of bugs for people expecting the existing behavior.

I could imagine maybe having optional_to and optional_cc exports, but it seems like overkill for something you can do pretty easily on the caller's side:

build_email(
    ( @to ? to(@to) : () ),
    ( @cc ? cc(@cc) : () ),
    ...
)
autarch commented 6 years ago

I forgot to say thanks for the suggestion and the PR.

alranel commented 6 years ago

This seems like it could easily lead to a lot of bugs for people expecting the existing behavior.

Can you elaborate on "lots of bugs"? What's a scenario do you think this PR would disrupt? The only affected scenario would be when someone is supplying no arguments and relying on Courriel to croak, which would be a very unusual pattern for checking an array for emptiness.

autarch commented 6 years ago

By "a lot of bugs" I mean that changing something from throwing an exception to not throwing one can easily break code. I don't think relying on a library to continue to throw an exception is unusual.