 | |
Re: [bug #7474] SSQLS compare operators patch
mailing.database.mysql-plusplus mailing.database.mysql-plusplus
 |
| | Thread Tools | Display Modes |
#1
 
| |||
| |||
|
Re: [bug #7474] SSQLS compare operators patch -
11-10-2006
, 08:54 PM
|
Summary: SSQLS compare operators patch Project: MySQL++ Submitted by: breeeze Submitted on: Пятница 20.10.2006 at 06:34 Category: None Priority: 5 - Normal Severity: 3 - Normal Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open __________________________________________________ _____ Details: I guess it'll be better decision to store all patches at gna.org This is a small patch to custom.pl, which using normal compare operators. |
this patch is. Why is the change necessary? What problem does it
solve? Can you give a test case showing how it improves the library, or
allows something to be done that couldn't be done previously?
--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw
#2
| |||
| |||
|
|
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 |
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.
--
MySQL++ Mailing List
For list archives: http://lists.mysql.com/plusplus
To unsubscribe: http://lists.mysql.com/plusplus?unsu...ie.nctu.edu.tw
|
« Previous Thread
|
Next Thread »
| Thread Tools | |
| Display Modes | |
Linear Mode
| |
Powered by vBulletin Version 3.5.3
Copyright ©2000 - 2012, Jelsoft Enterprises Ltd.
Copyright ©2000 - 2012, Jelsoft Enterprises Ltd.