This method allows me to filter by date a data structure list of hash that contains articles. I would like to have your opinion on the code:=method article_by_date    $articles->articles_by_date( month => 12 );Return a list of articles filter by date specified:    input: month (Int) : optional, month to match           year (Int) : optional, year to match    output: Hashref: Details of article=cutsub articles_by_date {    state $check = compile(        $invocant,        slurpy Dict[            month => Optional[Int],            year  => Optional[Int],        ]    );    my ($self, $arg) = $check->(@_);    my $month = $arg->{month};    my $year  = $arg->{year};    if ( ! defined($year) ) {        my $dt = DateTime->now;        $year  = $dt->year;    }    my $date  = defined($month)        ? qr/^\d\d\/$month\/$year\s\d\d:\d\d:\d\d$/        : qr/^\d\d\/\d\d\/$year\s\d\d:\d\d:\d\d$/;    my @articles;    foreach my $article ( @{$self->_get_or_create_cache('articles')} ) {        $article->{date} =~ $date            and push(@articles, $article);    }    return \@articles;}
Filter by date method
datetime;perl
Your code is mostly very readable and good. I have to point out the explicit type checking  this is wonderful!Minor Style RecommendationsThere are a few style thingies where I don't really care what you do, but it is common to put a comma after the last item in a list when it is at the end of a line:        ...,        slurpy Dict[            month => Optional[Int],            year  => Optional[Int],        ], # <-- HERE    );Also, the $article->{date} =~ $date    and push(@articles, $article);looks goofy:TIMTOWTDI, but that may look better with an if:push(@articles, $article) if $article->{date} =~ $date;I prefer to enclose regex applications in /.../, even when this is not syntactically neccessary: The delimiters make it visually clearer:push(@articles, $article) if $article->{date} =~ /$date/;This does not force recompilation of the regex on sufficiently modern perls (the last time I checked).Naming ImprovementsLet's move on to names. I do not know the purpose of your class. But if the class name, the object name, and all method names contain article, something may be wrong. (Of course _get_or_create_cache only contains article as an argument, but that seems to be effectively a part of the name).Assuming the purpose of the class is some collection of articles, then maybe the following may look more natural:my $articles = Articles->new(...);my $articles_from_may = $articles->at_date(month => 3);  # instead of articles_by_date# instead of _get_or_create_cache('articles'):# array dereference overloadingfor my $article (@$articles) {  ...; }# or a simpler methodfor my $article ($articles->all) {  ...;}But I could be misunderstanding the intentions of the class.As this method is all about dates, I would expect a $date variable to be some kind of date object or string. Nope, it is a regex. I do not know where you stand on Hungarian Notation, but sometimes it can really aid readability: $date_rx or something. Just don't overuse it.Major Style IssuesNow on to more serious stuff.Define-Or //Your if ( ! defined($year)) ... is a big waste of screen space. You can use the defined-or operator for shorter initialization:my $year = $arg->{year} // DateTime->now->year;Regex Formatting and CorrectnessYour two regexes are two lines of unreadable mess. The hypnotic rhythm of backslashes is only broken by the variable interpolations. But I can't tell at a glance what is happening here! We can introduce nonmatching space with the /x flag to make it more readable. You should also change the delimiter so that you don't have to escape the forward slashes.If there is any chance the date fields in your $article may be generated by an outside source, then the \d is wrong. This character class matches all Unicode digits, or [0-9] in some special circumstances. Either force ASCII semantics via /a flag, or be explicit with your character classes.my $time_rx = qr/ [0-9]{1,2} : [0-9]{1,2} : [0-9]{1,2} /x;my $date_rx = defined($month)    ? qr!\A [0-9]{1,2}/$month/$year     \s+ $time_rx \z!x    : qr!\A [0-9]{1,2}/[0-9]{1,2}/$year \s+ $time_rx \z!x;You should also heed Vedran ego's advice on this topic.BugsRegex Bug: Date FormattingNow there is a subtle bug here. What happens when the date string is 01/02/2013 13:46:02 and the user calls your method like month => 2? The regex would match 01/2/2013 ... but not /02/! This would mean that you have to sprintf '%02d' the stuff which you'd like to interpolate.This still assumes that you normalize all your dates (e.g. to UTC), so that your complete omission of time zones if forgivable. (Actually, it isn't. See below for some failure modes).Rant: Correct Date HandlingArticles Should Contain DateTimesHandling dates correctly is very complex. Yes, you can achieve some kind of seemingly working code with regexes, but I wouldn't recommend this. It would be preferable for each $article to include a DateTime object (just create them lazily if it seems too expensive to you). Then instead of the regex application:$article->date->year == $year && $article->date->month == $monthThis is shorter, more readable, and more correct, but more expensive than the regex solutions.The Method Should Have A DateTime As ArgumentThis still isn't completely correct, because no timezone information for the $year and $month is available. It might be better to require the user to call your method with a DateTime object, which you truncate to the month. Then:...my ($self, $date) = &$check;$date = $date->clone->truncate(to => 'month');...$article->date->year == $date->year && ...Don't Guess The Year!This would get rid of most bugs, and ambiguity. The latter exists because you try to guess a year. This can be bad UX:Assume that on Jan. 1st, I ask for all articles from December. Suprisingly, the query returns no articles, although there was a new article just yesterday!Or even better, assume that I ask for all articles from December during the last day of December. However, your program thinks it's already January, and returns an empty array.Finally, A SolutionI am not sure, but I think that...my $date_min = $date->clone->truncate(to => 'month');my $date_max = $date_min->clone->add(months => 1);...$date_min <= $article->date && $article->date < $date_maxmight work without bugs, assuming that the DateTime your method is called with has specified a correct time zone.Make map, not for.Finally, your foreach loop is really just a grep in disguise. You want:my @articles = grep { $date_min <= $_->date && $_->date < $date_max } $self->all;