[Kmymoney-devel] Re: Review Request: csvplugin update to add additional unit test, and improve field delimiter handling

[Allan Anderson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5463/
-----------------------------------------------------------

(Updated 2010-11-30 00:19:35.317522)


Review request for kmymoney.


Changes
-------

During further testing and live use, I found that certain unusual sequences of column entry combined with changes of choice failed properly to detect clashes of column numbers selected.

Because each column/field selection slot had its own error detection logic, the testing became difficult.  So, I've extracted most of that logic into a separate method, just leaving the slots to handle the UI side of things. (This applies to both of the main classes/UI - banking and investment.)  I've carried out extensive testing of both natural and 'unnatural' column selection.

I've also removed the remaining floats, replacing them with KMyMoneyMoney classes.  I've had to resort to using a double to produce a string suitable for QIF import, as I couldn't find a KMyMoneyMoney routine to do this easily.

The patch set is getting a little on the large side, and it would be good if these patches could be accepted soon so that any future patches are more easily checked. 


Summary
-------

The initial intention was to add an additional unit test, which involved removing data line parsing from the displayLine() routine into a small separate class to enable access from the unit test routine.  This led to benefits in two other areas.  

One csv test file I have had a comma as thousand separator, which was conflicting with the comma field separator.  Initially, I looked at this from the point of view of field separators and concocted a separator which successfully dealt with the issue.  However, I realised that the same odd separator might not work in another similar situation.  So, I've improved the detection and handling of this issue.

This then led to changes in csvProcessing() and investProcessing(), removing redundant code and improving efficiency slightly.

Two UIs have had a now unneeded field separator combobox item removed.  Some minor purely cosmetic changes also have been made.


Diffs (updated)
-----

  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/CMakeLists.txt 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/convdate.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvdatetest.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlg.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvimporterdlgdecl.ui 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvprocessing.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvprocessing.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvutil.h PRE-CREATION 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/csvutil.cpp PRE-CREATION 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlg.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investmentdlgdecl.ui 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investprocessing.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/investprocessing.cpp 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/parsedatatest.h PRE-CREATION 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/parsedatatest.cpp PRE-CREATION 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.h 1202162 
  /trunk/extragear/office/kmymoney/kmymoney/plugins/csvimport/redefinedlg.cpp 1202162 

Diff: http://svn.reviewboard.kde.org/r/5463/diff


Testing
-------

The new parsedatatest() unit test runs successfully.  Krazy and astyle have been run. Functional testing done with various csv format files.

Also, built and tested on a Ubuntu box, as well as openSuse. 800x600 and 1280x1024 resolutions checked, and different font sizes tested.

In use for my production.


Thanks,

Allan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kmymoney-devel/attachments/20101130/6b5bfcf3/attachment.htm