On Wed, Aug 28, 2024 at 8:34 PM Arrigo Marchiori <ard...@apache.org> wrote:

> Hello All,
>
> one small question for Damjan:
>
> [...]
>
> > commit 7b2bc0e6bba2fbc38d078306fe10d875115d6c86
> > Author: Damjan Jovanovic
> > AuthorDate: Mon Jul 22 08:06:52 2024 +0200
>
> [...]
>
> > diff --git a/main/connectivity/source/drivers/file/quotedstring.cxx
> b/main/connectivity/source/drivers/file/quotedstring.cxx
> > index 4ea452613b..366036b6f7 100644
> > --- a/main/connectivity/source/drivers/file/quotedstring.cxx
> > +++ b/main/connectivity/source/drivers/file/quotedstring.cxx
>
> [...]
>
> > @@ -91,49 +92,47 @@ namespace connectivity
> >      }
> >
> >      //------------------------------------------------------------------
> > -    void QuotedTokenizedString::GetTokenSpecial( String&
> _rStr,xub_StrLen& nStartPos, sal_Unicode cTok, sal_Unicode cStrDel ) const
> > +    void QuotedTokenizedString::GetTokenSpecial( ::rtl::OUString*
> _rStr,sal_Int32& nStartPos, sal_Unicode cTok, sal_Unicode cStrDel ) const
>
> Just out of curiosity: did you change the String& _rstr parameter into
> a _pointer_ to ::rtl::OUString for a reason? I would have used a
> _reference_ (::rtl::OUString&) instead, due to the fact that you
> dereference that pointer and never assume it could be NULL.
>
> The only positive consequence of this change I can think of, is that
> code calling this method will surely not compile, and so you can
> easily spot where this method is used.
>
> Did I guess right? :-)
>

Hi

The biggest change wasn't from a reference to a pointer. It was from the
mutable tools "String" to the immutable ::rtl::OUString. The previous code
also edited the string in place, which is no longer possible.

_rStr is an output parameter, overwritten with the value intended to be
returned. Maybe it would have been better to return that value, instead of
setting the parameter to it.

Oh and also, the thing I hate about C++ references, is that they make it
impossible to tell which argument will be passed by value and which by
reference, whereas a pointer makes it clear because of the "&" prefix. C#
has "out" and "ref" for that purpose. Rust has "mut". Every language after
C++ avoided that big mistake.


>
> Best regards,
> --
> Arrigo
>
>
Regards
Damjan

Reply via email to