Review Request 129014: Separate banking page

[Łukasz Wojniłowicz]

> On Wrz 25, 2016, 6:49 po południu, Thomas Baumgart wrote:
> > kmymoney/plugins/csvimport/bankingwizardpage.cpp, line 93
> > <https://git.reviewboard.kde.org/r/129014/diff/1/?file=477461#file477461line93>
> >
> >     I'd play with the findChildren<QComboBox*>() here to run this in a loop and doing the clear() and addItems() call inside the loop while having the columnNumbers stringlist set up before. See http://doc.qt.io/qt-5/qobject.html#findChildren for more details.

I think we'll lost on flexibility doing it with findChildren. What if in the future, on this page, there will be QComboBox, which won't need clear() and addItems(). That's the case in e.g. investmentwizardpage.cpp.
So besides having shorter code, is there really big advantage of doing so here?


- Łukasz


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129014/#review99515
-----------------------------------------------------------


On Wrz 25, 2016, 3:50 po południu, Łukasz Wojniłowicz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129014/
> -----------------------------------------------------------
> 
> (Updated Wrz 25, 2016, 3:50 po południu)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> -------
> 
> csvwizard.cpp has many lines of code, so it's good to separate banking
> page from it. The outcome is:
> 
> 1) m_pageBanking is initialized only when it's needed,
> 2) combo boxes on m_pageBanking are initialized only when needed,
> 3) removed registered fields, because there is no need to use them on
> other pages,
> 4) merged csvdialog with bankingwizardpage, so there is no ping-pong
> communication between them,
> 5) used only one slot for every combo box,
> 6) removed redundant includes,
> 7) tidied up names and structure of investmentwizardpage.
> 
> 
> Diffs
> -----
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 46cdc16 
>   kmymoney/plugins/csvimport/bankingwizardpage.h PRE-CREATION 
>   kmymoney/plugins/csvimport/bankingwizardpage.cpp PRE-CREATION 
>   kmymoney/plugins/csvimport/csvdialog.h d1e935c 
>   kmymoney/plugins/csvimport/csvdialog.cpp 9385d9b 
>   kmymoney/plugins/csvimport/csvimporterplugin.h e2908e4 
>   kmymoney/plugins/csvimport/csvutil.h 108718e 
>   kmymoney/plugins/csvimport/csvwizard.h 6fbb858 
>   kmymoney/plugins/csvimport/csvwizard.cpp 7f59af7 
> 
> Diff: https://git.reviewboard.kde.org/r/129014/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20160927/7a0f5f5e/attachment.html>