Code review comment for lp:~posulliv/drizzle/cleanup-replace-typelib

Padraig O'Sullivan wrote:
> Padraig O'Sullivan has proposed merging
> lp:~posulliv/drizzle/cleanup-replace-typelib into lp:drizzle.
>
> Requested reviews: Drizzle-developers (drizzle-developers)
>
> This patch replaces an instance of TYPELIB with std::vector. I looked
> for a simple, self-contained instance of TYPELIB to start with and I
> found one in client/drizzleadmin.cc. I wanted to start on a small and
> simple instance like this one so that I can get feedback on whether I
> am going about this the right way or not. I'm hoping to work on
> replacing more instances of TYPELIB throughout the code in the next
> few weeks.
>
> I'm looking to get comments on whether what I did to replace TYPELIB
> in this case is the best way to go about it. I wasn't sure if I
> should have gone with std::map or std::vector in this case. I found
> it easier to go with std::vector so that was my main motivation for
> using it here.
>

Looks good in general... but now I'm going to nitpick... :)

If I were you, I'd make command_names a string[] and command_vector a
vector of std::string. Then you can get rid of CommandMatch and just do:

static int get_command_type(const char *name)
{
  int type= ADMIN_ERROR;
  string comp_string= lower_string(name);
  vector<string>::iterator it=
    std::find(command_vector.begin(), command_vector.end(),
              comp_string);
  if (it != command_vector.end())
  {
    /* add 1 due to the way the commands ENUM is defined */
    type= distance(command_vector.begin(), it) + 1;
  }
  return type;
}

Monty