[Kde-pim] [patch] Printing emails [version 2]

[Ingo Klöcker]

On Wednesday 09 July 2008, Jaroslaw Staniek wrote:
> Thomas McGuire said the following, On 2008-07-08 18:43:
> > Please go for a signal/slot solution.
>
> Attached for review.
>
> - added KHtmlPartHtmlWriter::finished() signal, connected to
> KMReaderWin::slotPrintMsg().
> - previous KMReaderWin::printMsg() code splitted into two pieces:
>    1st - KMReaderWin::printMsg( KMMessage* aMsg ) - connects the
> finished() signal and calls setMsg(),
>    2nd - KMReaderWin::slotPrintMsg() - is a response for finished()
> signal, calls mViewer->view()->print() and deletesLater() the
> KMReaderWin.
>
> - at KMPrintCommand level, we keep QPointer<KMReaderWin> s_printerWin
>    globally until the KMReaderWin object is destroyed;
> - on KMPrintCommand::execute() we call
> s_printerWin->htmlWriter()->reset() to stop any previous processing

Using a static object is a no-go.

+  if ( s_printerWin && s_printerWin->htmlWriter() )
+    s_printerWin->htmlWriter()->reset(); // stop any previous 
processing

This will cause problems if the user manages to print twice in rapid 
succession because the second print job will kill the first one.

In fact, I do not see why the static variable is necessary at all. It is 
only used in a single method so keeping the pointer to the KMReaderWin 
in a variable appears to be superfluous. You can simply create several 
KMReaderWin on the heap without keeping the pointer. The KMReaderWin's 
will destroy themselves at the end of slotPrintMsg().


> - added KMReaderWin::mPartHtmlWriter which is needed for connecting
> it's signal to a slot

I do not really like the fact that mPartHtmlWriter points to the same 
object as mHtmlWriter (except if KMAIL_READER_HTML_DEBUG is defined). 
The lines
   delete mHtmlWriter; mHtmlWriter = 0;
+  mPartHtmlWriter = 0;
in KMReaderWin::~KMReaderWin() made me think mPartHtmlWriter might be 
leaked. I had to check the code to understand why this is not the case. 
I suggest to make mPartHtmlWriter a QPointer<KHtmlPartHtmlWriter>. This 
will make the above line
+  mPartHtmlWriter = 0;
obsolete and it will make it more obvious that mPartHtmlWriter is a weak 
pointer (i.e. it does not need to be deleted because somebody else is 
responsible for the deletion).


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080711/9070ab66/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/