← Back to team overview

maria-developers team mailing list archive

Refactoring in LEX_STRING and LEX_CSTRING

 

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".



Follow ups