Convert NaiveDate::from_weekday_of_month to return Result

[pitdicker]

A small PR converting just a single method.

In my personal use I found the way the method currently works inconvenient.

It is not uncommon for scheduling applications to equal some recurring thing on the 5th Weekday of the month with the last Weekday of the month. No matter if that turns out to be the 4th or the 5th Weekday.

Is that practise something we want to support? And if so do we want to adjust this method or add another one?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.02%. Comparing base (45d22e8) to head (776392a).

Additional details and impacted files

@@           Coverage Diff           @@
##            0.5.x    #1512   +/-   ##
=======================================
  Coverage   94.02%   94.02%           
=======================================
  Files          37       37           
  Lines       16974    16980    +6     
=======================================
+ Hits        15960    15966    +6     
  Misses       1014     1014           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[djc]

So I guess these are ordered alphabetically, and I haven't really reviewed for this before, but IMO it would be nice to order these more semantically, roughly in the order they get checked? Like, it feels like InvalidArgument should go first here.

[pitdicker]

I did not order them alphabetically, but by how often I expect users to encounter them.
DoesNotExist should be quite normal, as it totally depends on the calendar. InvalidArgument shows the input is not checked for correctness before, which I think is less common. OutOfRange is for dates so many thousands of years into the past or future that I think them to be are real edge cases.

Does that sound like a reasonable order?

[djc]

It is not uncommon for scheduling applications to equal some recurring thing on the 5th Weekday of the month with the last Weekday of the month. No matter if that turns out to be the 4th or the 5th Weekday.

It feels like that should be a separate method, last_weekday_of_month()?

(I guess an alternative would be to use i == 0 as a sentinel, but that seems generally harder to understand.)

[pitdicker]

i == 5 is the sentinel in common use. 👍 on making it a separate method.