[Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

> On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
> >

Revised patch to follow.  I've also attempted to fix the 'quirks' I referred to above, but I'll keep them separate from this, for clarity.


> On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
> > kmymoney/dialogs/transactioneditor.cpp, line 722
> > <http://git.reviewboard.kde.org/r/113143/diff/1/?file=199630#file199630line722>
> >
> >     Please declare variables at the point of their initialization, there is no point in declaring the number variable outside of the if statement. And use a const reference if you don't intend to change it.
> >     
> >     const QString &number = tr.splits().front().number();

On being passed to keepNewNumber(), 'number' was getting changed so couldn't be const.
I've reworked that area anyway, and dispensed with QString number.


> On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
> > kmymoney/mymoney/mymoneyfile.h, line 1480
> > <http://git.reviewboard.kde.org/r/113143/diff/1/?file=199631#file199631line1480>
> >
> >     Please pass a 'const QString &' as a parameter instead of by-value.
> >     And why does 'strNumericPart' only forward to the 'numericPart' private function, why don't you make 'numericPart' public and remove 'strNumericPart'?

> Please pass a 'const QString &' as a parameter instead of by-value. - Done.

I did originally have 'numericPart' as public, but had an impression that member variables were best not public so changed it.  Lack of formal tuition rears its head again. - Done.


On Oct. 10, 2013, 5:02 a.m., Allan Anderson wrote:
> > The above are just some plain old C++ coding style suggestions. Also should 'numericPart' really belong to the mymoneyfile object? If so I would add some testcases for it in the mymoneyfiletest suite.

No, not really.  I placed it there only because there were a couple of of other similar routines there, but they aren't really that similar.  As it's used only by TransactionEditor, I've moved it to there.


- Allan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/#review41476
-----------------------------------------------------------


On Oct. 7, 2013, 8:10 p.m., Allan Anderson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113143/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2013, 8:10 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 319801
>     http://bugs.kde.org/show_bug.cgi?id=319801
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> If a user's sequence of check numbers is broken by, say 'ATM' or an invoice number such as 'No 123-001 ABC', the next check number produced will be '1', entries containing alpha or punctuation characters not being saved.
> The fix corrects this by saving the complete entry, and uses any entered numeric part to calculate the next number in sequence.  If an existing numeric entry is edited, this entry will be taken into account for the next number.
> 
> There are some quirks.  If the entry which led to the current 'next number' is deleted, it is not possible to revert to the previous, now forgotten, 'next number', so the produced 'next number' is likely to leave a 'gap', and may need editing.  Also, there is no check that a new 'next number' does not already exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next number' will be the expected 25.  However, if the user corrects the error by changing a 23 to 22, the new 'next number' will be 23, which also already exists.  I'm not sure if such issues, which exist also in the current release, are worthy of fixing for a fairly unimportant area, without becoming more involved.
> 
> 
> Diffs
> -----
> 
>   kmymoney/dialogs/transactioneditor.h f07dafb 
>   kmymoney/dialogs/transactioneditor.cpp 39049cf 
>   kmymoney/mymoney/mymoneyfile.h af0c6fb 
>   kmymoney/mymoney/mymoneyfile.cpp ff7302c 
> 
> Diff: http://git.reviewboard.kde.org/r/113143/diff/
> 
> 
> Testing
> -------
> 
> Many manual entries checked, including coping with all values in the unit tests.  The unit test runs OK.
> 
> 
> Thanks,
> 
> Allan Anderson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20131010/5372a9a2/attachment.html>