in reply to Re: Get DoB Bounds from Age Range
in thread Get DoB Bounds from Age Range

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.

Replies are listed 'Best First'.
Re^3: Get DoB Bounds from Age Range
by wrog (Friar) on May 07, 2015 at 17:57 UTC
    Re
    suitable for use in SQL queries, e.g., WHERE dob < ? AND dob > ?
    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 < ?
    (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;
    except in my world (with half-open intervals) it's
    my $max = shift || ($min + 1);
    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).

      I have updated the function with an attempt to put your comments into action.

      Thank you for your comments.

Re^3: Get DoB Bounds from Age Range
by jeffa (Bishop) on May 07, 2015 at 17:38 UTC

    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('-'); }

    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;
    to this:
    my $max = shift || $min;
    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

    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)
    
      The 'science' behind my statement is that treating undefined as 0 for age will result in dates for 0 years, which probably isn't what the user wants.