Commit 296c528b authored by Tushar Maheshwari's avatar Tushar Maheshwari Committed by Albert Astals Cid
Browse files

Fix KHMThemeFactory leaks and KHMTheme const-correctness

- Replace allocation using `new KHMTheme` in `KHMThemeFactory::doTheme` and `KHMThemeFactory::buildTheme` (rename to `getTheme`).
- Add `const` to `KHMTheme` and `KHMThemeFactory` member functions not modifying data.
- Fix `const` usage in callers of `KHMThemeFactory::getTheme`.
- Miscellaneous:
  - syntax changes for iteration.
  - QString comparison using operator==.
  - Move local variable declaration close to first use.

Testing:
- Tested patch with local build: themes are listed and applied successfully.
parent 0a9af726
Pipeline #177386 passed with stage
in 1 minute and 3 seconds
......@@ -142,7 +142,7 @@ void KHangMan::setCurrentLanguage(int index)
void KHangMan::setCurrentTheme(int index)
{
KHMTheme *theme = m_themeFactory.buildTheme(index);
const KHMTheme *theme = m_themeFactory.getTheme(index);
Prefs::setTheme(theme->name());
Prefs::self()->save();
Q_EMIT currentThemeChanged();
......@@ -373,9 +373,7 @@ int KHangMan::currentTheme()
QString KHangMan::backgroundUrl()
{
QStringList themes = m_themeFactory.getNames();
int index = themes.indexOf(Prefs::theme());
KHMTheme *theme = m_themeFactory.buildTheme(index);
const KHMTheme *theme = m_themeFactory.getTheme(currentTheme());
if (theme) {
QString filename = QStandardPaths::locate(QStandardPaths::AppLocalDataLocation, QStringLiteral("themes/") + theme->svgFileName());
return filename;
......@@ -387,9 +385,7 @@ QColor KHangMan::currentThemeLetterColor()
{
// Default to white letters
QColor color = "white";
QStringList themes = m_themeFactory.getNames();
int index = themes.indexOf(Prefs::theme());
KHMTheme *theme = m_themeFactory.buildTheme(index);
const KHMTheme *theme = m_themeFactory.getTheme(currentTheme());
if (theme) {
color = theme->letterColor();
}
......
......@@ -44,75 +44,30 @@ KHMTheme::KHMTheme( const QString &name, const QString &uiName, const QString &s
{
}
QString KHMTheme::name()
{
return KHMname;
}
QString KHMTheme::uiName()
QString KHMTheme::uiName() const
{
return i18n(KHMuiName.toLatin1().constData());
}
QString KHMTheme::svgFileName()
{
return KHMsvgFileName;
}
QColor KHMTheme::letterInputTextColor()
{
return KHMletterInputTextColor;
}
QString KHMTheme::getAuthor()
{
return KHMauthor;
}
QString KHMTheme::getThemeVersion()
{
return KHMthemeVersion;
}
QRect KHMTheme::wordRect(const QSize& windowsize)
QRect KHMTheme::wordRect(const QSize& windowsize) const
{
return QRect(windowsize.width()*KHMwordRect.x()/10000, windowsize.height()*KHMwordRect.y()/10000,
windowsize.width()*KHMwordRect.width()/10000, windowsize.height()*KHMwordRect.height()/10000);
}
QRect KHMTheme::hintRect(const QSize& windowsize)
QRect KHMTheme::hintRect(const QSize& windowsize) const
{
return QRect(windowsize.width()*KHMhintRect.x()/10000, windowsize.height()*KHMhintRect.y()/10000,
windowsize.width()*KHMhintRect.width()/10000, windowsize.height()*KHMhintRect.height()/10000);
}
QRect KHMTheme::kRect(const QSize& windowsize)
QRect KHMTheme::kRect(const QSize& windowsize) const
{
return QRect(windowsize.width()*KHMkRect.x()/10000, windowsize.height()*KHMkRect.y()/10000,
windowsize.width()*KHMkRect.width()/10000, windowsize.height()*KHMkRect.height()/10000);
}
QColor KHMTheme::letterColor()
{
return KHMletterColor;
}
QColor KHMTheme::guessButtonTextColor()
{
return KHMguessButtonTextColor;
}
QColor KHMTheme::guessButtonColor()
{
return KHMguessButtonColor;
}
QColor KHMTheme::guessButtonHoverColor()
{
return KHMguessButtonHoverColor;
}
QPoint KHMTheme::goodWordPos(const QSize& windowsize, const QPoint& popupPos) //works good
QPoint KHMTheme::goodWordPos(const QSize& windowsize, const QPoint& popupPos) const //works good
{
return QPoint(popupPos.x() + windowsize.width()*KHMgoodWordPos.x()/10000,
popupPos.y() + windowsize.height()*KHMgoodWordPos.y()/10000);
......
......@@ -30,7 +30,7 @@ class KHMTheme
QRect KHMwordRect, KHMhintRect, KHMkRect;
QColor KHMletterColor, KHMguessButtonTextColor, KHMguessButtonColor, KHMguessButtonHoverColor, KHMletterInputTextColor;
QPoint KHMgoodWordPos;
public:
KHMTheme( const QString &name, const QString &uiName, const QString &svgFileName, const QString &author, const QString &themeVersion,
QRect wordRect, QRect hintRect, QRect kRect,
......@@ -38,35 +38,34 @@ class KHMTheme
QPoint goodWordPos);
///The name of theme as in the folder
QString name();
QString name() const { return KHMname; }
///The name of the theme in the menu
QString uiName();
QString uiName() const;
///Get the svg n22 the theme
QString svgFileName();
QString svgFileName() const { return KHMsvgFileName; }
///Set the position and size for drawing the word to guess
QRect wordRect(const QSize& windowsize);
QRect wordRect(const QSize& windowsize) const;
QRect hintRect(const QSize& windowsize);
QRect hintRect(const QSize& windowsize) const;
///Set the position and size for drawing the hanged K
QRect kRect(const QSize& windowsize);
QRect kRect(const QSize& windowsize) const;
///Set the color for the word and the missed letters
QColor letterColor();
QColor letterColor() const { return KHMletterColor; }
///Set the color of the Guess word
QColor guessButtonTextColor();
QColor guessButtonTextColor() const { return KHMguessButtonTextColor; }
///Set the color of the Guess button background
QColor guessButtonColor();
QColor guessButtonColor() const { return KHMguessButtonColor; }
///Set the color of the Guess button background when the mouse is over it
QColor guessButtonHoverColor();
QColor guessButtonHoverColor() const { return KHMguessButtonHoverColor; }
///Set the color of the input text in the input widget
QColor letterInputTextColor();
QColor letterInputTextColor() const { return KHMletterInputTextColor; }
///Set the already guessed popup position
QPoint goodWordPos(const QSize& windowsize, const QPoint& popupPos);
QPoint goodWordPos(const QSize& windowsize, const QPoint& popupPos) const;
//Find out who's the author
QString getAuthor();
QString getAuthor() const { return KHMauthor; }
//Version is equal to a KHMTheme format version declared in XML file
QString getThemeVersion();
QString getThemeVersion() const { return KHMthemeVersion; }
};
#endif
......
......@@ -20,18 +20,9 @@
#include "khmthemefactory.h"
#include <QDebug>
KHMThemeFactory::KHMThemeFactory()
{
}
bool KHMThemeFactory::addTheme(const QString &themeFile)
{
QFile file (themeFile);
QDomDocument tree(QStringLiteral("KHangManTheme")); //do we need it?
QDomElement root;
QDomNode themeNode;
QDomElement themeElement;
if (!file.exists()) {
qDebug() << "Themes file doesn't exist";
return false;
......@@ -42,6 +33,7 @@ bool KHMThemeFactory::addTheme(const QString &themeFile)
return false;
}
QDomDocument tree(QStringLiteral("KHangManTheme")); //do we need it?
if (!tree.setContent(&file)) {
file.close();
qDebug()<<"Can't set content for theme file";
......@@ -49,33 +41,28 @@ bool KHMThemeFactory::addTheme(const QString &themeFile)
}
file.close();
root=tree.documentElement();
const QDomElement root=tree.documentElement();
if (!checkTheme(root, QStringLiteral("1"))) {
qDebug()<<"Incompatible version of theme loaded";
return false;
}
QString themeVersion=root.attribute(QStringLiteral("version"));
themeNode=root.firstChild();
do {
const QString themeVersion=root.attribute(QStringLiteral("version"));
for (QDomNode themeNode=root.firstChild(); !themeNode.isNull(); themeNode=themeNode.nextSibling()) {
doTheme(themeNode.toElement(), themeVersion);
themeNode=themeNode.nextSibling();
} while (!themeNode.isNull());
}
return true;
}
void KHMThemeFactory::walkDirectory(const QDir &dir) //unused! (but works)
{
QFileInfoList xmlFilesList;
QStringList allowedExtenstion(QStringLiteral("*.xml"));
if (dir.exists()) {
xmlFilesList=dir.entryInfoList(allowedExtenstion, QDir::Files);
const QStringList allowedExtenstion(QStringLiteral("*.xml"));
const QFileInfoList xmlFilesList=dir.entryInfoList(allowedExtenstion, QDir::Files);
for (QFileInfoList::iterator i=xmlFilesList.begin(); i!=xmlFilesList.end(); ++i) {
addTheme(i->absoluteFilePath());
for (const auto &file : xmlFilesList) {
addTheme(file.absoluteFilePath());
}
}
}
......@@ -85,37 +72,35 @@ int KHMThemeFactory::getQty() const
return themesList.size();
}
QStringList KHMThemeFactory::getNames()
QStringList KHMThemeFactory::getNames() const
{
QStringList list;
for (int i=0; i<themesList.size(); i++) {
list.append(themesList[i].name());
for (const auto &theme : themesList) {
list.append(theme.name());
}
return list;
}
QStringList KHMThemeFactory::themeList()
QStringList KHMThemeFactory::themeList() const
{
QStringList list;
for (int i=0; i<themesList.size(); i++) {
list.append(themesList[i].uiName());
for (const auto &theme : themesList) {
list.append(theme.uiName());
}
return list;
}
KHMTheme * KHMThemeFactory::buildTheme (int id)
const KHMTheme * KHMThemeFactory::getTheme(int id) const
{
if (id >= 0 && id < themesList.size())
return new KHMTheme(themesList[id]);
else
return nullptr;
if (id >= 0 && id < themesList.size()) {
return &themesList[id];
}
return nullptr;
}
QRect KHMThemeFactory::makeRect(const QDomElement &element, const QString &propertyName)
{
QDomElement rect=element.firstChildElement(propertyName);
const QDomElement rect=element.firstChildElement(propertyName);
return QRect(
(int)(rect.attribute(QStringLiteral("xratio")).toDouble()*10000),
......@@ -127,50 +112,44 @@ QRect KHMThemeFactory::makeRect(const QDomElement &element, const QString &prope
QColor KHMThemeFactory::getColor(const QDomElement &element, const QString &propertyName)
{
QDomElement color=element.firstChildElement(propertyName);
const QDomElement color=element.firstChildElement(propertyName);
return QColor ( color.attribute(QStringLiteral("r")).toInt(), color.attribute(QStringLiteral("g")).toInt(), color.attribute(QStringLiteral("b")).toInt());
}
bool KHMThemeFactory::checkTheme(const QDomElement &root, const QString &themeVersion)
{
QString rootName=root.tagName();
const QString rootName=root.tagName();
return (rootName.compare(QLatin1String("KHangManThemes"))==0) && (themeVersion.compare(root.attribute(QStringLiteral("version")))==0);
return (rootName==QLatin1String("KHangManThemes")) && (themeVersion==root.attribute(QStringLiteral("version")));
}
void KHMThemeFactory::doTheme(const QDomElement &theme, const QString &version) //fetch all the properties from .xml and stick it together
//"theme" means <theme></theme> section
{
QDomElement coords=theme.firstChildElement(QStringLiteral("coords"));
//Names of elements are camelCase, but
//attributes are always lowercase
QString name=theme.attribute(QStringLiteral("name"));
QString uiName=theme.attribute(QStringLiteral("uiname"));
QString svgFileName=theme.attribute(QStringLiteral("svgfilename"));
QString author=theme.attribute(QStringLiteral("author"));
QString themeVersion=version;
QColor letterColor=getColor(theme.firstChildElement(QStringLiteral("colors")), QStringLiteral("letterColor"));
QColor guessButtonTextColor=getColor(theme.firstChildElement(QStringLiteral("colors")), QStringLiteral("guessButtonTextColor"));
QColor guessButtonColor=getColor(theme.firstChildElement(QStringLiteral("colors")), QStringLiteral("guessButtonColor"));
QColor guessButtonHoverColor=getColor(theme.firstChildElement(QStringLiteral("colors")), QStringLiteral("guessButtonHoverColor"));
QColor letterInputTextColor=getColor(theme.firstChildElement(QStringLiteral("colors")), QStringLiteral("letterInputTextColor"));
const QString name=theme.attribute(QStringLiteral("name"));
const QString uiName=theme.attribute(QStringLiteral("uiname"));
const QString svgFileName=theme.attribute(QStringLiteral("svgfilename"));
const QString author=theme.attribute(QStringLiteral("author"));
QRect wordRect=makeRect(theme.firstChildElement(QStringLiteral("coords")), QStringLiteral("wordRect"));
QRect hintRect=makeRect(theme.firstChildElement(QStringLiteral("coords")), QStringLiteral("hintRect"));
QRect kRect=makeRect(theme.firstChildElement(QStringLiteral("coords")), QStringLiteral("kRect"));
const QDomElement colors=theme.firstChildElement(QStringLiteral("colors"));
const QColor letterColor=getColor(colors, QStringLiteral("letterColor"));
const QColor guessButtonTextColor=getColor(colors, QStringLiteral("guessButtonTextColor"));
const QColor guessButtonColor=getColor(colors, QStringLiteral("guessButtonColor"));
const QColor guessButtonHoverColor=getColor(colors, QStringLiteral("guessButtonHoverColor"));
const QColor letterInputTextColor=getColor(colors, QStringLiteral("letterInputTextColor"));
QDomElement wordPos = theme.firstChildElement(QStringLiteral("coords")).firstChildElement(QStringLiteral("goodWordPos"));
QPoint goodWordPos = QPoint( wordPos.attribute(QStringLiteral("wratio")).toDouble()*10000, wordPos.attribute(QStringLiteral("hratio")).toDouble()*10000 );
const QDomElement coords=theme.firstChildElement(QStringLiteral("coords"));
const QRect wordRect=makeRect(coords, QStringLiteral("wordRect"));
const QRect hintRect=makeRect(coords, QStringLiteral("hintRect"));
const QRect kRect=makeRect(coords, QStringLiteral("kRect"));
KHMTheme * newTheme = new KHMTheme (name, uiName, svgFileName, author, themeVersion, wordRect, hintRect, kRect, letterColor, guessButtonTextColor, guessButtonColor, guessButtonHoverColor, letterInputTextColor, goodWordPos);
themesList.append(*newTheme);
}
const QDomElement wordPos = coords.firstChildElement(QStringLiteral("goodWordPos"));
const QPoint goodWordPos = QPoint( wordPos.attribute(QStringLiteral("wratio")).toDouble()*10000, wordPos.attribute(QStringLiteral("hratio")).toDouble()*10000 );
KHMThemeFactory::~KHMThemeFactory()
{
themesList.clear();
const KHMTheme newTheme (name, uiName, svgFileName, author, version, wordRect, hintRect, kRect, letterColor, guessButtonTextColor, guessButtonColor, guessButtonHoverColor, letterInputTextColor, goodWordPos);
themesList.append(newTheme);
}
......@@ -34,23 +34,20 @@ class KHMTheme;
class KHMThemeFactory
{
public:
KHMThemeFactory();
~KHMThemeFactory();
bool addTheme(const QString &filePath); //returns "true" if theme has been added successfully, "false" otherwise
void walkDirectory(const QDir &dir); //walks the directory loads valid themes files. No recursion
int getQty() const; //returns quantity of list
QStringList getNames(); //returns short names(worknames) of all the themes
QStringList themeList(); //returns user interface names of all the themes
KHMTheme * buildTheme(int id); //Returns theme at "index". An "index" must exists
QStringList getNames() const; //returns short names(worknames) of all the themes
QStringList themeList() const; //returns user interface names of all the themes
const KHMTheme * getTheme(int id) const; //Returns theme at "index". An "index" must exists
private:
QList<KHMTheme> themesList;
QRect makeRect(const QDomElement &element, const QString &propertyName);
QColor getColor(const QDomElement &element, const QString &propertyName);
bool checkTheme(const QDomElement &root, const QString &themeVersion);
static QRect makeRect(const QDomElement &element, const QString &propertyName);
static QColor getColor(const QDomElement &element, const QString &propertyName);
static bool checkTheme(const QDomElement &root, const QString &themeVersion);
void doTheme(const QDomElement &theme, const QString &version);
};
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment