[Kmymoney-devel] Re: Review Request: Accounts model optimization

[Cristian Onet]

> On March 6, 2011, 2:52 p.m., Thomas Baumgart wrote:
> > /trunk/extragear/office/kmymoney/kmymoney/models/accountsmodel.cpp, line 763
> > <http://svn.reviewboard.kde.org/r/6589/diff/3/?file=45471#file45471line763>
> >
> >     As discussed this should also not happen.

Yes, it does not happen in the case we discussed but after that I decided to implement a hierarchy change in the "modified" signal with a remove followed by an add in which case it is possible to have sub accounts.


> On March 6, 2011, 2:52 p.m., Thomas Baumgart wrote:
> > /trunk/extragear/office/kmymoney/kmymoney/models/accountsmodel.cpp, line 767
> > <http://svn.reviewboard.kde.org/r/6589/diff/3/?file=45471#file45471line767>
> >
> >     Ah, and here you mention it!

Then I have to remove this, see my previous comment.


> On March 6, 2011, 2:52 p.m., Thomas Baumgart wrote:
> > /trunk/extragear/office/kmymoney/kmymoney/views/kmymoneyview.cpp, line 552
> > <http://svn.reviewboard.kde.org/r/6589/diff/3/?file=45475#file45475line552>
> >
> >     Why do you disconnect the signals here? If no storage is attached to the engine no signals should be sent out anyway. Also during the initial load of a new file, the objectXxx signals should not be emitted. If they do, then it's a problem I need to fix in the engine.

If that is the case then I'll remove the calls to disconnect.


> On March 6, 2011, 2:52 p.m., Thomas Baumgart wrote:
> > /trunk/extragear/office/kmymoney/kmymoney/models/models.cpp, line 90
> > <http://svn.reviewboard.kde.org/r/6589/diff/3/?file=45473#file45473line90>
> >
> >     An easier method I used at another spot is to simply set an empty model to be used by the views in such cases.

Technically that is exactly what is done now but by removing only the rows the columns are kept as they should be.


- Cristian


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


On March 5, 2011, 9:19 p.m., Cristian Onet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6589/
> -----------------------------------------------------------
> 
> (Updated March 5, 2011, 9:19 p.m.)
> 
> 
> Review request for kmymoney.
> 
> 
> Summary
> -------
> 
> Instead of synchronizing the whole account tree on each data changed take advantage of the new, more granular, signals and perform the appropriate modifications in the model's data after it has been loaded once.
> 
> Also create a separate instance for the institutions model. This change has the following rationale. Mixing the two hierarchies in the same model (by account type and institution) would make it difficult to implement changes since depending on the changes it would imply structural modifications in only one instance of the same account. So for example changing the institution means a structural change in the institution hierarchy but no change in the accounts by type hierarchy. Separating the two hierarchies means that one account can be found only once in the model and it also simplifies the implementation of the modifications slots.
> 
> 
> This addresses bug 230277.
>     https://bugs.kde.org/show_bug.cgi?id=230277
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/office/kmymoney/kmymoney/models/accountsmodel.h 1223871 
>   /trunk/extragear/office/kmymoney/kmymoney/models/accountsmodel.cpp 1223871 
>   /trunk/extragear/office/kmymoney/kmymoney/models/models.h 1223871 
>   /trunk/extragear/office/kmymoney/kmymoney/models/models.cpp 1223871 
>   /trunk/extragear/office/kmymoney/kmymoney/views/kinstitutionsview.cpp 1223871 
>   /trunk/extragear/office/kmymoney/kmymoney/views/kmymoneyview.cpp 1223871 
> 
> Diff: http://svn.reviewboard.kde.org/r/6589/diff
> 
> 
> Testing
> -------
> 
> Simple testing using a test file: adding/removing/modifying accounts and modifying banances.
> 
> 
> Thanks,
> 
> Cristian
> 
>

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