Alligator in KDEReview

[Ingo Klöcker]

On Sonntag, 1. November 2020 12:29:43 CET Albert Astals Cid wrote:
> El dijous, 29 d’octubre de 2020, a les 0:34:30 CET, Tobias Fella va 
escriure:
> > Since I'm still a beginner at programming with Qt/cpp/qml, I'd
> > appreciate comments or suggestions on my code ;)
> 
> You're missing KLocalizedString::setApplicationDomain in your main.cpp
> 
> You're not deleteing the Author you create on Entry::Entry, i'm guessing a
> qDeleteAll(m_authors); in the destructor is what you want.
> 
> Similarly you seem to also need a qDeleteAll(m_entries) in EntriesModel
> destructor

I strongly suggest not to use raw pointers in C++ code unless you really need 
to (e.g. if you create millions of objects and cannot afford the performance 
or memory cost). In all other cases you should delegate the object life-time 
to C++.

Either use one of the STL smart pointers [1], or, if you insist on Qt, one of 
the Qt "equivalents" like QSharedPointer or QScopedPointer.

In the case of Author it would probably be much better to make Author a value-
class. After all, Author is basically just a
struct {
  QString name;
  QString email;
  QString url;
}

Or does it need to be a heavy QObject because of QML? If yes, then you could 
also make use of Qt's parent-child-life-cycle-management (see e.g. [2]) and 
delegate the deletion of the Author objects to Entry by passing `this` instead 
of `nullptr` as parent argument to Author's constructor, i.e. by doing
```
        m_authors += new Author(..., this);
```
instead of
```
        m_authors += new Author(..., nullptr);
```
By the way, explicitly passing `nullptr` for parent is pretty useless because 
`nullptr` is the default value for parent according to the definition of 
Author's constructor.

Regards,
Ingo

[1] https://en.cppreference.com/w/cpp/memory
[2] https://doc.qt.io/qt-5/objecttrees.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20201101/ac8e3089/attachment.sig>