Move away from using raw pointers

Removed the raw pointers and replaced them with std::shared_ptr

src/contentmanager.cpp

@@ -16,6 +16,7 @@
 #include <zim/item.h>
 #include <QHeaderView>
 #include "contentmanagerdelegate.h"
+#include "node.h"
 #include "rownode.h"
 #include "descriptionnode.h"
 #include "kiwixconfirmbox.h"
@@ -236,13 +237,8 @@ void ContentManager::openBookWithIndex(const QModelIndex &index)
 {
  try {
  QString bookId;
- if (index.parent().isValid()) {
- auto bookNode = static_cast<DescriptionNode*>(index.internalPointer());
- bookId = bookNode->getBookId();
- } else {
- auto bookNode = static_cast<RowNode*>(index.internalPointer());
- bookId = bookNode->getBookId();
- }
+ auto bookNode = static_cast<Node*>(index.internalPointer());
+ bookId = bookNode->getBookId();
  // check if the book is available in local library, will throw std::out_of_range if it isn't.
  KiwixApp::instance()->getLibrary()->getBookById(bookId);
  if (getBookInfos(bookId, {"downloadId"})["downloadId"] != "")

src/contentmanagermodel.cpp

@@ -2,12 +2,6 @@
 #include "node.h"
 #include "rownode.h"
 #include "descriptionnode.h"
-#include<QList>
-#include<QMap>
-#include<QDebug>
-#include <QStringList>
-#include <QSize>
-#include <QIcon>
 #include <zim/error.h>
 #include <zim/item.h>
 #include "kiwixapp.h"
@@ -20,7 +14,6 @@ ContentManagerModel::ContentManagerModel(QObject *parent)
 
 ContentManagerModel::~ContentManagerModel()
 {
- delete rootNode;
 }
 
 int ContentManagerModel::columnCount(const QModelIndex &parent) const
@@ -35,7 +28,7 @@ QVariant ContentManagerModel::data(const QModelIndex& index, int role) const
  if (!index.isValid())
  return QVariant();
 
- Node *item = static_cast<Node*>(index.internalPointer());
+ auto item = static_cast<Node*>(index.internalPointer());
  const auto displayRole = role == Qt::DisplayRole;
  const auto additionalInfoRole = role == Qt::UserRole+1;
  if (displayRole || additionalInfoRole)
@@ -61,14 +54,14 @@ QModelIndex ContentManagerModel::index(int row, int column, const QModelIndex &p
  RowNode* parentItem;
 
  if (!parent.isValid()) {
- parentItem = rootNode;
+ parentItem = rootNode.get();
  }
  else {
  parentItem = static_cast<RowNode*>(parent.internalPointer());
  }
  auto childItem = parentItem->child(row);
  if (childItem)
- return createIndex(row, column, childItem);
+ return createIndex(row, column, childItem.get());
 
  return QModelIndex();
 }
@@ -78,13 +71,13 @@ QModelIndex ContentManagerModel::parent(const QModelIndex &index) const
  if (!index.isValid())
  return QModelIndex();
 
- Node *childItem = static_cast<Node*>(index.internalPointer());
- Node *parentItem = childItem->parentItem();
+ auto childItem = static_cast<Node*>(index.internalPointer());
+ auto parentItem = childItem->parentItem();
 
- if (parentItem == rootNode)
+ if (!parentItem || parentItem == rootNode)
  return QModelIndex();
 
- return createIndex(parentItem->row(), 0, parentItem);
+ return createIndex(parentItem->row(), 0, parentItem.get());
 }
 
 int ContentManagerModel::rowCount(const QModelIndex &parent) const
@@ -111,7 +104,7 @@ QVariant ContentManagerModel::headerData(int section, Qt::Orientation orientatio
 void ContentManagerModel::setBooksData(const QList<QMap<QString, QVariant>>& data)
 {
  m_data = data;
- rootNode = new RowNode({tr("Icon"), tr("Name"), tr("Date"), tr("Size"), tr("Content Type"), tr("Download")}, "", nullptr);
+ rootNode = std::shared_ptr<RowNode>(new RowNode({tr("Icon"), tr("Name"), tr("Date"), tr("Size"), tr("Content Type"), tr("Download")}, "", std::weak_ptr<RowNode>()));
  setupNodes();
  emit dataChanged(QModelIndex(), QModelIndex());
 }
@@ -162,7 +155,7 @@ void ContentManagerModel::refreshIcons()
 
 bool ContentManagerModel::hasChildren(const QModelIndex &parent) const
 {
- Node *item = static_cast<Node*>(parent.internalPointer());
+ auto item = static_cast<Node*>(parent.internalPointer());
  if (item)
  return item->childCount() > 0;
  return true;
@@ -211,15 +204,24 @@ void ContentManagerModel::sort(int column, Qt::SortOrder order)
 
 void ContentManagerModel::updateImage(QModelIndex index, QString url, QByteArray imageData)
 {
- RowNode *item = static_cast<RowNode*>(index.internalPointer());
+ if (!index.isValid())
+ return;
+ auto item = static_cast<RowNode*>(index.internalPointer());
+ if (!rootNode->isChild(item))
+ return;
  item->setIconData(imageData);
  iconMap[url] = imageData;
  emit dataChanged(index, index);
 }
 
+std::shared_ptr<RowNode> getSharedPointer(RowNode* ptr)
+{
+ return std::static_pointer_cast<RowNode>(ptr->shared_from_this());
+}
+
 void ContentManagerModel::startDownload(QModelIndex index)
 {
- auto node = static_cast<RowNode*>(index.internalPointer());
+ auto node = getSharedPointer(static_cast<RowNode*>(index.internalPointer()));
  node->setIsDownloading(true);
  auto id = node->getBookId();
  QTimer *timer = new QTimer(this);

src/contentmanagermodel.h

@@ -6,9 +6,11 @@
 #include <QVariant>
 #include <QIcon>
 #include "thumbnaildownloader.h"
+#include <memory>
 
 class RowNode;
 class Node;
+class DescriptionNode;
 
 class ContentManagerModel : public QAbstractItemModel
 {
@@ -46,7 +48,7 @@ public slots:
 
 private:
  QList<QMap<QString, QVariant>> m_data;
- RowNode *rootNode;
+ std::shared_ptr<RowNode> rootNode;
  int zimCount = 0;
  ThumbnailDownloader td;
  QMap<QString, QByteArray> iconMap;

src/descriptionnode.cpp

@@ -1,21 +1,27 @@
 #include "descriptionnode.h"
 #include "rownode.h"
 
-DescriptionNode::DescriptionNode(QString desc, RowNode *parent)
+DescriptionNode::DescriptionNode(QString desc, std::weak_ptr<RowNode> parent)
  : m_desc(desc), m_parentItem(parent)
 {}
 
 DescriptionNode::~DescriptionNode()
 {}
 
-Node* DescriptionNode::parentItem()
+std::shared_ptr<Node> DescriptionNode::parentItem()
 {
- return m_parentItem;
+ std::shared_ptr<Node> temp = m_parentItem.lock();
+ if (!temp)
+ return nullptr;
+ return temp;
 }
 
 QString DescriptionNode::getBookId() const
 {
- return m_parentItem->getBookId();
+ std::shared_ptr<RowNode> temp = m_parentItem.lock();
+ if (!temp)
+ return QString();
+ return temp->getBookId();
 }
 
 int DescriptionNode::childCount() const
@@ -37,5 +43,8 @@ QVariant DescriptionNode::data(int column)
 
 int DescriptionNode::row() const
 {
- return m_parentItem->row();
+ std::shared_ptr<RowNode> temp = m_parentItem.lock();
+ if (!temp)
+ return 0;
+ return temp->row();
 }

src/descriptionnode.h

@@ -9,9 +9,9 @@ class RowNode;
 class DescriptionNode : public Node
 {
 public:
- DescriptionNode(QString desc, RowNode *parent);
+ DescriptionNode(QString desc, std::weak_ptr<RowNode> parent);
  ~DescriptionNode();
- Node *parentItem() override;
+ std::shared_ptr<Node> parentItem() override;
  int childCount() const override;
  int columnCount() const override;
  QVariant data(int column) override;
@@ -20,7 +20,7 @@ class DescriptionNode : public Node
 
 private:
  QString m_desc;
- RowNode *m_parentItem;
+ std::weak_ptr<RowNode> m_parentItem;
 };
 
 #endif // DESCRIPTIONNODE_H

src/node.h

@@ -2,12 +2,13 @@
 #define NODE_H
 
 #include <QVariant>
+#include <memory>
 
-class Node
+class Node : public std::enable_shared_from_this<Node>
 {
 public:
  virtual ~Node() = default;
- virtual Node *parentItem() = 0;
+ virtual std::shared_ptr<Node> parentItem() = 0;
  virtual int childCount() const = 0;
  virtual int columnCount() const = 0;
  virtual QVariant data(int column) = 0;

src/rownode.cpp

@@ -4,7 +4,7 @@
 #include "descriptionnode.h"
 #include "kiwix/tools.h"
 
-RowNode::RowNode(QList<QVariant> itemData, QString bookId, RowNode *parent)
+RowNode::RowNode(QList<QVariant> itemData, QString bookId, std::weak_ptr<RowNode> parent)
  : m_itemData(itemData), m_parentItem(parent), m_bookId(bookId)
 {
  m_downloadInfo = {0, "", "", false};
@@ -13,12 +13,12 @@ RowNode::RowNode(QList<QVariant> itemData, QString bookId, RowNode *parent)
 RowNode::~RowNode()
 {}
 
-void RowNode::appendChild(Node *item)
+void RowNode::appendChild(std::shared_ptr<Node> item)
 {
  m_childItems.append(item);
 }
 
-Node *RowNode::child(int row)
+std::shared_ptr<Node> RowNode::child(int row)
 {
  if (row < 0 || row >= m_childItems.size())
  return nullptr;
@@ -35,9 +35,12 @@ int RowNode::columnCount() const
  return 6;
 }
 
-Node* RowNode::parentItem()
+std::shared_ptr<Node> RowNode::parentItem()
 {
- return m_parentItem;
+ std::shared_ptr<Node> temp = m_parentItem.lock();
+ if (!temp)
+ return nullptr;
+ return temp;
 }
 
 QVariant RowNode::data(int column)
@@ -49,13 +52,20 @@ QVariant RowNode::data(int column)
 
 int RowNode::row() const
 {
- if (m_parentItem)
- return m_parentItem->m_childItems.indexOf(const_cast<RowNode*>(this));
+ try {
+ std::shared_ptr<RowNode> temp = m_parentItem.lock();
+ if (temp) {
+ auto nodePtr = std::const_pointer_cast<Node>(shared_from_this());
+ return temp->m_childItems.indexOf(nodePtr);
+ }
+ } catch(...) {
+ return 0;
+ }
 
  return 0;
 }
 
-RowNode* RowNode::createNode(QMap<QString, QVariant> bookItem, QMap<QString, QByteArray> iconMap, RowNode *rootNode)
+std::shared_ptr<RowNode> RowNode::createNode(QMap<QString, QVariant> bookItem, QMap<QString, QByteArray> iconMap, std::shared_ptr<RowNode> rootNode)
 {
  auto faviconUrl = "https://" + bookItem["faviconUrl"].toString();
  QString id = bookItem["id"].toString();
@@ -72,12 +82,26 @@ RowNode* RowNode::createNode(QMap<QString, QVariant> bookItem, QMap<QString, QBy
  bookIcon = iconMap[faviconUrl];
  }
  }
- auto temp = new RowNode({bookIcon, bookItem["title"],
+ std::weak_ptr<RowNode> weakRoot = rootNode;
+ auto rowNodePtr = std::shared_ptr<RowNode>(new
+ RowNode({bookIcon, bookItem["title"],
  bookItem["date"],
  QString::fromStdString(kiwix::beautifyFileSize(bookItem["size"].toULongLong())),
  bookItem["tags"]
- }, id, rootNode);
- auto tempsTemp = new DescriptionNode(bookItem["description"].toString(), temp);
- temp->appendChild(tempsTemp);
- return temp;
+ }, id, weakRoot));
+ std::weak_ptr<RowNode> weakRowNodePtr = rowNodePtr;
+ const auto descNodePtr = std::make_shared<DescriptionNode>(DescriptionNode(bookItem["description"].toString(), weakRowNodePtr));
+ rowNodePtr->appendChild(descNodePtr);
+ return rowNodePtr;
+}
+
+bool RowNode::isChild(Node *candidate)
+{
+ if (!candidate)
+ return false;
+ for (auto item : m_childItems) {
+ if (candidate == item.get())
+ return true;
+ }
+ return false;
 }

src/rownode.h

@@ -18,27 +18,28 @@ struct DownloadInfo
 class RowNode : public Node
 {
 public:
- explicit RowNode(QList<QVariant> itemData, QString bookId, RowNode *parentItem);
+ explicit RowNode(QList<QVariant> itemData, QString bookId, std::weak_ptr<RowNode> parentItem);
  ~RowNode();
- Node *parentItem() override;
- Node *child(int row);
- void appendChild(Node *child);
+ std::shared_ptr<Node> parentItem() override;
+ std::shared_ptr<Node> child(int row);
+ void appendChild(std::shared_ptr<Node> child);
  int childCount() const override;
  int columnCount() const override;
  QVariant data(int column) override;
  int row() const override;
- QString getBookId() const { return m_bookId; }
+ QString getBookId() const override { return m_bookId; }
  void setIconData(QByteArray iconData) { m_itemData[0] = iconData; }
  bool isDownloading() const { return m_isDownloading; }
  void setDownloadInfo(DownloadInfo downloadInfo) { m_downloadInfo = downloadInfo; }
  DownloadInfo getDownloadInfo() const { return m_downloadInfo; }
  void setIsDownloading(bool val) { m_isDownloading = val; }
- static RowNode* createNode(QMap<QString, QVariant> bookItem, QMap<QString, QByteArray> iconMap, RowNode *rootNode);
+ static std::shared_ptr<RowNode> createNode(QMap<QString, QVariant> bookItem, QMap<QString, QByteArray> iconMap, std::shared_ptr<RowNode> rootNode);
+ bool isChild(Node* candidate);
 
 private:
  QList<QVariant> m_itemData;
- QList<Node*> m_childItems;
- RowNode *m_parentItem;
+ QList<std::shared_ptr<Node>> m_childItems;
+ std::weak_ptr<RowNode> m_parentItem;
  QString m_bookId;
  bool m_isDownloading = false;
  DownloadInfo m_downloadInfo;