[Kmymoney-devel] KMyMoney: proposal to replace hard coded utf-8 in KReportsView::KReportTab

Alvaro Soliverez asoliverez at gmail.com
Fri Apr 16 22:42:54 CEST 2010


Hello Bernd,
see my answers below.

On Fri, Apr 16, 2010 at 5:13 PM, Bernd Gonsior <bgo at freeplexx.de> wrote:
> Alvaro Soliverez schrieb am 14.04.2010 05:05:
>> On Sun, Apr 11, 2010 at 12:10 PM, Bernd Gonsior <bgo at freeplexx.de> wrote:
>> > Hello Alvaro,
>> >
>> > the new class is ready for review.
>> >
>> > Changes at a glance:
>> > - html generation is moved from KReportsView to KReportsViewHtml
>>
>> there is no need to create a new class. the code to create the new
>> class should be in ReportTable.
>
> Here i need some clarification. Class ReportTable is described as 'interface
> definition' and currently contains only virtual members. I could add the
> methods of KReportsViewHtml to ReportTable as normal, non-virtual members in
> following ways:
>
> 1. as simple methods

As simple methods.
The public method renderHTML will call protected methods renderHeader,
renderBody, renderFooter, etc.
renderBody is the method that will be implemented by the inheriting classes.


> 2. as a new inner class of ReportTable containing all methods
> 3. as inner class of your standard inner class 'Private'
>
> What would you prefer? Maybe you prefer another way?
>
>
>> A new file reporttable.cpp could hold that code, since it's used by
>> all child classes.
>
> OK
>
>> In fact, the current renderHTML() method of the child classes could be
>> renamed to renderBody(), and be called from the ReportTable. So,
>> KReportView should only call createTable of the child classes, and it
>> would execute all what's needed.
>
> OK, this would influence following sources:
>
> views/kreportsview.cpp
> reports/pivottable.h
> reports/querytabletest.cpp
> reports/listtable.h
> reports/reporttable.h
> reports/pivottable.cpp
> reports/reportstestcommon.cpp
> reports/listtable.cpp
>

Ok.

>
>> > - KFileDialog in KReportsView::slotSaveView did not show the includeCss
>> >  checkbox, fixed
>> > - state of file filter for KFileDialog in KReportsView::slotSaveView was
>> > not kept between successive calls, fixed
>>
>> You deleted the Private class. The FileSaveProperties class should
>> actually be within that Private class and not instead of it. That aids
>> in library compatibility.
>
> OK, i had a glimpse at your developer-doc but i did not find something about
> this (the doc seems to be a bit out of date). Where could i have a look for
> things like that?
>

Here is a techbase page on private classes.
http://techbase.kde.org/Policies/Library_Code_Policy#D-Pointers

>> Next time, please try to separate the patches when they fix more than
>> one stuff to make it easier for review.
>
> OK, there will be 3 patches:
>
> 1. 'make includeCss checkbox appear'
>
> 2. 'separate html generation', to check all functionality of this patch no. 1
>   is needed
>
> 3. 'rename renderHTML() to renderBody()'
>

Ok. You can make 2 and 3 into one patch, since it's all part of the same change.


More information about the KMyMoney-devel mailing list