[Kmymoney-devel] Patch for HolidayRegion and Working Week

[Colin]

On Wednesday January 6 2010 08:32 pm Ian Neal wrote:
> Hi all,
> 
> Find attached a patch (gzipped due to size) for the current kmymoney svn
> which makes the following changes:
> * Switch from MoveFriday/MoveMonday to MoveBefore/MoveAfter for use with
> scheduled transactions;
> * Strings with Friday / Monday now use previous / next working day;
> * Strings with Weekend now use non-working days;
> * Removed duplicate code from kenterscheduledlg.cpp and got it to use
> the code from mymoneyscheduled.cpp;
> * Added UI to scheduled transactions settings page to be able to select
> relevant holiday calendar that the OS knows about as well as a (None)
> option;
> * Added routine to calculate working week from KGlobal::locale() to
> kmymoney.cpp;
> * Added methods to mymoneyscheduled.cpp to read/write holiday region and
> working week for scheduled transactions;
> * Added defaults for holiday region (gb) and working week
> (Monday-Friday) to mymoneyscheduled.cpp;
> * In mymoneyscheduled.cpp split some of the code out of
> adjustedNextDueDate to create adjustedDate so that kenterscheduledlg.cpp
> could use it instead of duplicating code;
> * Rewrote the code in adjustedDate to it reflects the locale's working
> week and the selected holiday region;
> * Added a new test that makes sure the working week is set correctly and
> working/non-working days are identified correctly;
> * Added further tests to testAdjustedNextDueDate to make sure that the
> adjusted date is correct for non-working days not just weekends;
> * Corrected it's vs its in various comments throughout code.
> 
> KHoliday assumes that any day marked as red or weekend is a holiday and
> at the moment it has no concept of financial holidays so I've worked
> with what it has provided.
> 
> Any constructive comments/feedback/review are welcome,
> 
> TIA,
> 
> Ian
> 



My comments:
* Not tested it, but it looks great, Ian.  I've been wanting to see calendar 
support for some time.
* I'd default Schedule working works and holidays using the settings instead 
of hard-coding to British.  If you must hard-code I'd default to USA for 
consistency: I know it can be wearing but defaulting to US make particular 
sense given the tighter integration with KDE.
* I'd prefer to have a separate Calendar class with work weeks and holidays.
  * You'd be able to assign the same Calendar to multiple Schedules.
  * You could have Working Day date math in the class: then we add "Every n 
Working Days" Occurrence
  * It would be easier to expand the Calendar functionality later to support 
user-defined Calendars to allow Schedules to deal with the variability of US 
public holidays and school holidays.

Thanks,

Colin