maria-developers team mailing list archive

[Vicențiu Ciorbaru <cvicentiu@xxxxxxxxx>]

Hi!

I would like to add a few more arguments in favour of using const
references and not const pointers. I do not endorse non-const references,
that makes the code silly and confusing a lot of the times. When we need
output or input-output parameters we should keep using pointers. For
strictly mandatory (must never be null) input parameters, const references
are the way to go, unless there is a strong reason not to.

The guarantee that references enforce is "never null". This is a very
important and useful feature of C++. We can completely skip any checks such
as:

func1(LEX_CSTRING *str)
{
   if (!str) { .. }
}

Or DBUG_ASSERTS such as:
func1(LEX_CSTRING *str)
{
    DBUG_ASSERT(str);
    ...
}

The code with *const references*  is self documenting in this sense. It
makes it very clear that the function does not own the object it is being
passed (no questions wether the pointer should be freed or not) and that
the reference is valid.

Other arguments, not strictly related to this use case, but are in favor of
const references:

* By limiting our use of references to const references we avoid the
ambiguity between regular pass-by-value arguments and non-const reference
arguments. You work with them as if they are pass-by-value. Since they are
const you can't do anything to them anyway, you're not changing any of the
caller's data.
* Pointers are unclear wether they point to a buffer array or a single
item. References do not allow you to do: ptr[0], ptr[1], ptr[2], etc.
* If we chose to make use of STL algorithms or C++11 features down the
line, references tend to work easier than pointers. Can't come up with a
good example right this moment, but most stl algorithms expect
reference-based parameters, ex:
std::max(*int_ptr_a, *int_ptr_b) vs std::max(int_a, int_b)
* If some legacy code does need a pointer parameter, using the unary &
operator on the reference will produce the required pointer type.

If we are doing refactoring anyway, I strongly request we reconsider the
policy of avoiding references completely. I suggest we at least make use of
them as input parameters to functions and methods in new code.

Vicențiu



On Thu, 20 Apr 2017 at 00:10 Alexander Barkov <bar@xxxxxxxxxxx> wrote:

> Hello all,
>
>
> Monty is now doing refactoring:
>
> 1. Replacing a lot of LEX_STRING to LEX_CSTRING.
>
> 2. Fixing functions and methods that have arguments of types:
> a. pointer "const LEX_STRING *name" and
> b. reference "const LEX_STRING &name".
> to the same style. Monty chose passing by pointer (2a).
>
>
> I think that:
>
> - #1 is good.
> - #2 is good, in the sense of changing to the same style.
>
> But I think that the choice of "passing by pointer" in #2 is bad.
> Passing by reference for LEX_STRING/LEX_CSTRING related things would be
> better.
>
>
> With the "passing by pointer" approach whenever one needs to pass
> a LEX_STRING to a function accepting LEX_CSTRING, one will have to do:
>
>
> void func1(const LEX_STRING *str)
> {
>   LEX_CSTRING tmp= {str->str, str->length};
>   func2(&tmp);
> }
>
>
>
>
> I'd propose two things instead:
>
> 1. Add a class:
>
>
> class Lex_cstring: public LEX_CSTRING
> {
> public:
>   Lex_cstring() {} /*Uninitialized */
>   Lex_cstring(const char *str, size_t length)
>   {
>     LEX_STRING::str= str;
>     LEX_STRING::length= length;
>   }
>   Lex_cstring(const LEX_STRING *str)
>   {
>     LEX_STRING::str= str->str;
>     LEX_STRING::length= str->length;
>   }
> };
>
>
> 2. Instead of "const LEX_CSTRING *str"
> we use "const Lex_cstring &str" all around C++ code.
>
> So the above example with passing a LEX_STRING to
> something that expect a LEX_CSTRING would be simplified to
>
> void func1(const LEX_STRING *str)
> {
>   func2(str);
> }
>
>
> 3. In some classes now we have duplicate methods and constructors like
> this:
>
> class XXX
> {
> public:
>   XXX(const char *str, size_t length) { ... }
>   XXX(const LEX_STRING *str) { ... }
>   void method(const char *str, size_t length) {...}
>   void method(const LEX_STRING *str) {...}
> };
>
> to make it possible to call like this:
>
>   new XXX(str, length);
>   new XXX(lex_str_ptr)
>   new XXX(&lex_str)
>   method(str, length);
>   method(lex_str_ptr);
>   method(&lex_str);
>
>
> We change this to "passing by reference" and remove duplicates:
>
> class XXX
> {
> public:
>   XXX(const Lex_cstring &str) { ... }
>   void method(const Lex_cstring &str) {...}
> };
>
> After this change we can call like this:
>
>   new XXX(Lex_cstring(str, length));
>   new XXX(*lex_str_ptr)
>   new XXX(lex_str);
>   method(Lex_cstring(str, length));
>   method(*lex_str_ptr);
>   method(lex_str);
>
> Notice, not needs for separate methods accepting
> two arguments "const char *str, size_t length".
>