Code review comment for lp:~posulliv/drizzle/replace-dynamic-array

"Overall, great! Couple tiny things... :)

1) Const correctness in functor.

The args are const correct, good, but you should also add const to the function declaration, too, since the state of the functor doesn't change via the operator():

change from:

inline bool operator()(const SHOW_VAR *var1, const SHOW_VAR *var2)

to:

inline bool operator()(const SHOW_VAR *var1, const SHOW_VAR *var2) const

Same comment as above for the show_var_remove_if functor's operator().

2) Put assignments on a separate line from comparisons.

You do:

+ int val;
+ if ((val = strcmp(var1->name, var2->name)) < 0)

It would be better to do:

+ int val= strcmp(var1->name, var2->name);
+ if (val < 0)

Generally, a single line of source code should try to do a single operation. There is no performance benefit from putting it in a single line, and doing the assignment on a separate line leads to less error-prone code.

You could even change the body of the operator() to just this:

int val= strcmp(var1->name, var2->name);
return (val < 0);

3)

- all_status_vars.elements--; // but next insert_dynamic should overwite it
+ all_status_vars.insert(all_status_vars.begin(), list++);

You have 3 spaces instead of 2 at the start of this line. Yeah, I know...I'm nitpicky. :)

Thanks for the comments. I addressed the 3 things you mentioned in the branch.