[Development] QList

[Marc Mutz]

On Thursday 23 March 2017 21:16:55 Thiago Macieira wrote:
> On quinta-feira, 23 de março de 2017 12:41:56 PDT Marc Mutz wrote:
> > On 2017-03-23 19:27, Thiago Macieira wrote:
> > > In the mean time: why do you care if some class derives from
> > > QStringView?
> > 
> > Because it's _wrong_.
> > 
> > It does not model is-a, so public inheritance is out of the question.
> 
> I beg to differ. It *is* "is-a": any modifiable QString is also a view to
> itself.

"is-a" is a terminus technicus. It means is-subtype-of aka. 
https://en.wikipedia.org/wiki/Liskov_substitution_principle

> Also, remember the point that it would be a private base, so the is-a
> relationship is not visible. This is about code reuse, not about the
> semantic.

If it's private inheritance, it's not is-a.

> > QString cannot use a lot of QStringView functions (like mid()) without
> > changing at least the return type, so inherit-to-reuse-implementation is
> > also not valid.
> 
> For some cases, that's true, but only because functions like mid() try to
> return *this if they detect that there's no change. That's an optimisation
> inside QString, not a requirement of the interface.
> 
> A naïve implementation of QString::mid() could be:
> 
> QString QString::mid(int start, int len) const
> {
> 	return QString(QStringView::mid(start, len));
> }

Which is no different from

   return QString(QStringView(*this).mid(start, len));

so does not provide a case supporting using inhertance.

> > Last, there are no virtual functions in QStringView that QString would
> > want to reimplement.
> 
> Virtual has nothing to do with anything. QString is complementing
> QStringView, only adding functionality.

I'm merely listing the use-cases for inheritance. Good that you agree that 
none appy :)

> > So, of the three use-cases for inheritance, none are relevant here, so
> > it follows that inheritance is not the tool to use. You may consider
> > composition, though, as I said, I'd rather provide free functions that
> > most QString/View methods forward to than picking one class and calling
> > its methods from all others.
> 
> From a strict theoretical point of view, that might be better.
> 
> From the practical point of view, implementing something as simple as
> 
> int QString::indexOf(QChar ch, int from, Qt::CaseSensitivity cs) const
> {
> 	return QStringView(*this).indexOf(c, from, cs);
> }

I said it many time, and I'll say it once more: The way to share code is 
through free functions, not one class delegating to another:

  return qFindChar(*this, ch, from, cs);

or

  return qFindString(*this, QStringView(&ch, 1), from, cs);

where only the qFoo() functions are exported, but none of the QString/View/Ref 
members.

> means:
> 1) we have an extra entry point in the library
> 2) so more symbols in the symbol table, increasing the chances of hash
>    collision
> 3) more code that, at best, inlines QStringView::indexOf into the new
>   function. That is, the best we can hope for is that the compiler turns
> the above into:
> 
> int QString::indexOf(QChar ch, int from, Qt::CaseSensitivity cs) const
> {
>     return findChar(unicode(), length(), ch, from, cs);
> }
> 
> Also note we'd probably want to mark findChar as Q_NEVER_INLINE so that the
> compiler doesn't expand the same function twice, which would mean an even
> bigger QtCore.

Note how all of those concerns are addressed by qCompareStrings()-like 
delegation to free functions.

> > > We certainly need to discuss the presence of an extra pointer inside,
> > > as that
> > > has a cost. But derivation?
> > 
> > Derivation is the strongest coupling mechanism in C++, after friendship.
> > Do I need to mention QPolygon and what pain it's inheritance from
> > QVector<QPoint> has caused? (FTR: cf. end of qvector.h and
> > qvector_msvc.cpp).
> 
> Because MSVC ABI, like the Windows ABI, has severe shortcomings and
> actually violate the C and C++ standards. Some of them are legacy reasons
> from the 1980s, like the one causing crashes reported in QTBUG-38876
> (attempt for Qt 6 fix at https://codereview.qt-project.org/189193 ).

I'm merely pointing out that inheriting one value class from another, even 
publicly (but the same problem would exist with private inheritance, I 
guess?), has already caused much pain. We should learn from it two things:

1. not inhert one value type from another
2. not class-level-export value classes, only export out-of-line functions

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt, C++ and OpenGL Experts