Korolyov Ilya wrote:
> Follow-up Comment #4, bug #7474 (project mysqlpp):
>
> In current implementation, compare looks like
>
> int cmp; \
> cmp = mysqlpp::sql_cmp(x.C1 , y.C1 ); \
> if (cmp) return cmp; \
> cmp = mysqlpp::sql_cmp(x.C2 , y.C2 );
>
> etc
>
> sql_cmp (custom.h) need general types, so it seems to me comparing for
> example
> sql_datetime, lead to 2 converts sql_datetime=>std::string (correct me, if I
>
> wrong).
>
> New implementation use
>
> bool operator == (const NAME &other) const{\
> bool cmp; \
> cmp = (C1 == other.C1 ); \
> if (cmp) return cmp; \
> cmp = (C2 == other.C2 ); \
> if (cmp) return cmp; \
> return (C3 == other.C3);\
> \
> }
>
> so, to compare sql_datetime, operator==() shoud be used without any
> conversion. I guess it should be faster
This is a fine idea, but the patch is way too messy right now. Several
concerns come to mind while perusing it:
1. How do you justify leaving $parm2, $define, $set and $compp blank?
They're still being used later on in compare.pl. As far as I can tell,
you have a choice between two courses. First, you can completely remove
the mechanisms that use these values, and then convince me that this is
a reasonable choice. I suspect you can remove all of sql_compare_*, but
I haven't looked that deeply into it, so you'll have to make a good
argument to get me to accept such a change. The second option is to
make a very conservative patch to just add your new operators, leaving
the existing behavior untouched. This makes the code bigger than it has
to be, but it requires less justification.
2. I don't understand the reason for "bool cmp" in the == operator
definition. Why can't it be defined like the other two operators?
3. The initial assignment of $comp_equal is ugly: you're setting it to
one of two different values based on a condition, but the assignments
are separated by unrelated code. This makes it hard to follow. I'd say
something like this instead:
$comp_equal = ($i == 1 ? " bool cmp; \\\n" : "");
4. It's a minor thing, but you've got some uneven use of whitespace.
You'll inspire more confidence in me with neatly formatted code. In my
experience, messy code is associated with messy engineering. You'll
have an easier time convincing me of the soundness of your code if it
looks good, too.
