[scribus] virtual destructors and other coding culture things

Nick Shaforostoff shaforostoff at kde.ru
Wed Jul 29 21:49:58 CEST 2009


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





More information about the scribus mailing list