Please mark gtParagraphStyle destructor as virtual,
because it is inherited by gtFrameStyle and thus creates potential memory 
leaks.

Also, looking through the code I saw usage of redundant if's, where functional 
style
could be used (this is faster in terms of code performance because modern CPUs
don't like if statements). I corrected few of those in my patch
(http://bugs.scribus.net/view.php?id=8308),
but please don't write redundant if's any more in the future.

-       if (isShown)
-               m_masterPagesPalette->show();
-       else
-               m_masterPagesPalette->hide();
+       m_masterPagesPalette->setVisible(isShown);



-       if (imgInfo.isEmbedded && useEmbedded)
-               imgInfo.isEmbedded = true;
-       else
-               imgInfo.isEmbedded = false;
+       q->imgInfo.isEmbedded=q->imgInfo.isEmbedded && useEmbedded;


-               if (sy < sx)
-                       image.createLowRes(sx);
-               else
-                       image.createLowRes(sy);
+               image.lowerResolution(qMax(sx, sy));

-       void setName(const QString& n)   { m_name = n.isEmpty() ? "" : n; }
+       void setName(const QString& n)   { m_name = n; }

and so on, also see 
http://bugs.scribus.net/file_download.php?file_id=4214&type=bug



***
I explicitly ask to apply the following diff to scribus*format_save.cpp files:
 void Scribus134Format::WritePages(ScribusDoc *doc, ScXmlStreamWriter& docu, 
QProgressBar *dia2, uint maxC, bool master)
 {
        uint ObCount = maxC;
-       Page *page;
-       uint pages;
        QDomElement pg;
        QString tmp;
-       if (master)
-               pages = doc->MasterPages.count();
-       else
-               pages = doc->DocPages.count();
-       for(uint i = 0; i < pages; ++i)
+    QString elementName=master?"MASTERPAGE":"PAGE";
+       foreach(Page *page, (master?doc->MasterPages:doc->DocPages))
        {
                ObCount++;
                if (dia2 != 0)
                        dia2->setValue(ObCount);
-               if (master)
-               {
-                       docu.writeStartElement("MASTERPAGE");
-                       page = doc->MasterPages.at(i);
-               }
-               else
-               {
-                       docu.writeStartElement("PAGE");
-                       page = doc->DocPages.at(i);
-               }
+               docu.writeStartElement(elementName);
+





As you can see all my proposed changes make code shorter and more readable



Reply via email to