[Kmymoney-devel] Patch for HolidayRegion and Working Week

[Thomas Baumgart]

On Thursday 07 January 2010 02:32:16 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,

OK, you asked for them, here they are:

* member variables should be named m_xxx not mXxxx to maintain consistency
* for new private members, the d-pointer architecture should be used (see 
http://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B for 
details - search for "d pointer")
* parts in and beyond the mymoney directory should only rely on Qt libs. I 
know that this is not given today and we are working on solving those issues. 
We should pay attention if we add a new dependancy. See below for more 
details.
* overall rating: clean stuff almost ready for integration

Since the only dependancy on KHolidays is the isHoliday(const QDate&) method, 
we could simplify the interface and remove the dependancy in those steps:

1) remove all references to KHoliday
2) create a static pointer to a bool isHoliday(const QDate&) function and 
initialize it to 0
3) create a setter for this function pointer 
   void setHolidayCheck(bool (*f)(const QDate&))
   I am not sure if we need a getter method for that pointer
4) Create a KHoliday object within the scope of the application
5) pass the address of the isHoliday(const QDate&) method to setHolidayCheck
6) upon deletion of the KHoliday object set the pointer within MyMoneySchedule 
to 0. Maybe the usage of a QSharedPointer for this object is useful
7) Only use the isHoliday function pointer in MyMoneySchedule if it is 
non-zero

Hope that is somewhat clear. If not, please ask and I try to clarify. If 
someone has a better alternative to solve the dependancy, let us know.

-- 

Regards

Thomas Baumgart

GPG-FP: E55E D592 F45F 116B 8429   4F99 9C59 DB40 B75D D3BA
-------------------------------------------------------------
morphir: so much confusion :S kmake, kdemake, qmake make cmake etc.
logixoul: you forgot cmakekde :)
morphir: and bakemeacake
-------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 224 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100109/68f48045/attachment.sig