If it works then it works. :) But since you asked, i would rename the variables and try to clean up how you handle default values. For example:

use DateTime;

# ...

sub DoBrangefromAges {
    my $min = shift || 0;
    my $max = shift || 0;
    my $inclusive = $_[0] ? 1 : 0;

    my ($xs,$ns) = (DateTime->now, DateTime->now);
    $xs->subtract(years => $min, days => -$inclusive);
    $ns->subtract(years => $max, days => 364 + $inclusive);

    return $xs->ymd('-'), $ns->ymd('-');
}
[download]

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

Thank you for your comments. I have improved the variable names. I have a bad habit of using very short variable names for small functions and things that won't be used very long.

However, I felt that your suggestions for default values would lead to unexpected results (that would also be difficult to understand) in the form of a valid query input that would give results that were obviously not what was intended.

Also, setting the default for maxage would remove the ability for users to provide just one age and get valid results (by having the function copy the minimum to the maximum).

I also decided, based on this thinking, to die instead of returning undef, because if the user is calling this function without parameters, how can we trust them to check for an undef return before passing it into the query? ;)

Thank you for your help in improving this function.

Re

 suitable for use in SQL queries, e.g., WHERE dob < ? AND dob > ?
[download]

Just as a matter of general programming style, I've found that life is improved if you get into the habit of always, always thinking in terms of half-open intervals and then write your code/libraries accordingly, e.g., here you expect that what people will actually want is to do queries of the form:

 suitable for use in SQL queries, e.g., WHERE ? <= dob AND dob < ?
[download]

(note also that this way the parameters have the lower bound first, which is what people generally expect; also this phrasing makes it more immediately visually obvious that dob is supposed to be between the two limits ... anything you can do to help the reader is a win).

That way, if you're doing two separate queries on "0 to 10" (which includes 0 but not 10) and "10 to 20" (which includes 10 but not 20) that gets you exactly "0 to 20" and you're not forgetting to include 10 or including it twice. Lots of fencepost errors go away without your having to think about it, and if you need to know how many things are in a range, you just subtract ("20 to 30" has 10 things in it because 30-20=10).

And then you get rid of the inclusive/exclusive parameter entirely (because if everybody else using the library follows this convention, then they can just add 1 day to either of the upper or lower bound dates they get back as they need to, and in their own code it'll be obvious what they're doing; but, chances are, they won't be needing to do that at all because the half-open interval will have been the Right Thing in the first place).

Also with respect to your defaults, I'm pretty sure the original respondent meant

my $max = shift || $min;
[download]

except in my world (with half-open intervals) it's

my $max = shift || ($min + 1);
[download]

which then makes it obvious that if you leave off the second parameter, you're going to get back a 1-year interval (which will have exactly 365 days in it because it will include, say, 2014-May-17 and not 2015-May-17).

Suggestions are suggestions: you cherry pick them according to your needs and tastes. :) However ... where is your science? Simply "feeling" that these default values will not work for is not the same as actually testing them out.

use strict;
use warnings;
use Test::More qw( no_plan );
use DateTime;

for my $min (1 .. 20) {
    for my $max (30 .. 80) {
        for my $inc ( 0 .. 1 ) {
            is_deeply
                [ jeffa( $min, $max, $inc ) ],
                [ over2sd( $min, $max, $inc ) ],
            ;
        }
    }
}

done_testing();


sub jeffa {
    my $min = shift || 0;
    my $max = shift || 0;
    my $inclusive = $_[0] ? 1 : 0;

    my ($xs,$ns) = (DateTime->now,DateTime->now);
    $xs->subtract(years => $min, days => -$inclusive);
    $ns->subtract(years => $max, days => 364 + $inclusive);

    return $xs->ymd('-'),$ns->ymd('-');
}

sub over2sd {
    my ($agemin,$agemax,$inclusive) = @_;
    die "Minimum age omitted in DoBrangefromAges" unless (defined $age
+min and $agemin ne '');
    $agemin = int($agemin);
    $agemax = int($agemin) unless defined $agemax;
    $agemax = int($agemax);
    $inclusive = ($inclusive ? $inclusive : 0);
    my ($maxdob,$mindob) = (DateTime->now,DateTime->now);
    $maxdob->subtract(years => $agemin, days => -$inclusive);
    $mindob->subtract(years => $agemax, days => 364 + $inclusive);
    return $maxdob->ymd('-'),$mindob->ymd('-');
}
[download]

When i run this test i see no difference in the output of your code and mine. It is certainly possible that i have not tested as fully as can be tested, but i have more certainty that i what i provided will give you the desired output, as was my goal.

UPDATE:
You only need change one line in my sub. From this:

my $max = shift || 0;
[download]

to this:

my $max = shift || $min;
[download]

Returning 0 if the user passes 0 arguments seems like justice to me. >:)

use Test::More qw( no_plan );

is_deeply
    [ jeffa( ) ],
    [ over2sd( ) ],
;

is_deeply
    [ jeffa( 10 ) ],
    [ over2sd( 10 ) ],
;

__END__
Use of uninitialized value $agemin in int at foo.pl line 37.
ok 1
ok 2
1..2
[download]

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

Do not call now() at all.

It's bad enough that you're calling now() twice, which could introduce really subtle bugs that only get triggered very intermittently (imagine someone using this function right at midnight).

Also, consulting the system clock is a system call and all system calls are potentially expensive in certain contexts; perhaps not this one, but you still want to get into the habit of thinking of the system clock as something you don't look at unless you need to.

And whoever is calling your function will need to be thinking about which "now()" they actually want to be using. E.g., in a webserver context there's a request time and, chances are, the framework has already made the system call for you, and life gets simpler if you take the position that everything w.r.t. processing that request is happening in a single instant.

That is a good point. Forcing^H^H^H^H^H^H^H Allowing the user to pass a reference date in also makes the function more flexible, letting them calculate ages at an arbitrary point in time.

Thank you.

please do not change your original post after people have been making comments on it, this makes the comments hard to understand at best. It is better to add an update section at the bottom of the post.