[mythtv] QString: pass by const& or value?

[Jean-Yves Avenard]

Personally, I like to know what is happening and how, and for readers
to understand what is going on at a glance.

For QString passing by value might be okay because ultimately it's a
refcounted object with copy on write and copy is cheap.

But for others not quite so.

So passing by const reference when you don't intend a transfer of
ownership or passing by rvalue if you do , gives a much greater
clarity and makes code review easier.
And when writing copy constructors, have both, either explicitly or
with = default.

e.g.
class MyClass {
  MyClass(const MyClass& other);
  MyClass(MyClass&& other);
};

On Sat, 23 Oct 2021 at 02:06, David Hampton via mythtv-dev
<mythtv-dev at mythtv.org> wrote:
>
> On Thu, 2021-10-21 at 00:45 -0400, Scott T wrote:
> > Is it necessary to pass QString by const reference?  Specifically I'm
> > looking at libmythmetadata/videometadata (converting to non-PIMPL) and
> > libmyth/programinfo (some unchanged by
> > https://github.com/MythTV/mythtv/commit/1a8097e3249a8ed2ea051056da08a35993bdc953
> >
> > )
> >
> > If I understand correctly, passing by value increments the reference
> > counter before the function call, and then using std::move just moves
> > the (pointer to the) data.  Thus, passing by value is recommended for
> > constructors.
>
> I think its six of one or half dozen of the other, with one caveat. If
> you pass by const reference, nothing has to be done in the caller and
> its the callee's responsibility to copy the object (which as you
> pointed out is a small object and a reference count to the data). If
> you pass by value, the caller has to copy the object and increment the
> reference count and the callee doesn't have to do anything. The caveat
> is that when the constructor implementation is in the header file and
> it uses call by value and std::move, then the optimizer has more
> opportunities to find things that it can optimize.
>
> Googling will get you arguments for both approaches. The clang-tidy
> docs say that now that std::move exists and many library functions have
> been updated with move constructors that pass by value with std::move
> is the preferred approach.
>
> I have no objection to converting constructors to use pass-by-value and
> std::move. Seems like a change that should be a standalone commit.
>
> Clazy also has a couple of checks related to pass by value/reference:
>
> https://github.com/KDE/clazy/blob/master/docs/checks/README-function-args-by-ref.md
> https://github.com/KDE/clazy/blob/master/docs/checks/README-function-args-by-value.md
>
> > Should QStrings be returned from a function by value or const reference
> > as many are in videometadata?
>
> I think I prefer const reference. The callee is just loading a pointer
> into a register, and then its up to the caller whether or not to make a
> copy of it. It you return a value, you always make a copy of the
> string.
>
> David
>
>
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev at mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org