Re: [SQLObject] A small patch around features I need

[G. <fra...@cl...>]

On 30 Jun 2003 03:37:30 -0500
Ian Bicking <ia...@co...> wrote:

> On Fri, 2003-06-27 at 11:42, Fran=E7ois Girault wrote:
> > Clarisys patch for SQLOBject 0.3
>=20
> There's several things that have changed since 0.3 -- it would be
> helpful to do this against CVS instead.

Ok, this is what i'm going to do now :)

> >   * for Postgresql usage only due to modification in DBConnection, and
> > I have no time to write & test with other DB (help welcome :) ).
>=20
> It's very helpful if you include a unit test for any new features --
> this is more important to me than supporting all the databases.

Ok I'll try to include that.

> A couple notes on the code itself:
>=20
> -        func =3D eval('lambda self: self._SO_joinList[%i].performJoin(se=
lf)' % index)
> +        func =3D eval('lambda self: self._SO_joinList[%s].performJoin(se=
lf)' % index)
>=20
> That %i isn't related to the ID.

Ok :)

> (in .new()):
> +       inst._inst_id =3D None
> +       if kw.has_key('id'):
> +           inst._inst_id =3D kw['id']
> +           del kw['id']
> +       else:
> +           if kw.has_key(inst._idName):
> +               inst._inst_id =3D kw[inst._idName]
> +               del kw[inst._idName]
>=20
> I think you're allowing people to use the database name of the column
> here, even though that's not allowed for any of the other columns.  I
> don't think that should be allowed, nor should it be generalized to the
> rest of the columns -- it just creates ambiguity in your class usage.

Ok, I agree about ambiguity. So I'll only keep 'id' field name parameter. B=
ut i have to store given id in the class otherwise I don't know how to give=
 this values as parameter in _SO_finishCreate() :

id =3D self._connection.queryInsertID(self._table, self._idName, names, val=
ues, self._inst_id)

maybe an additionnal method to _connection would be the solution.

> Anyway, I'd accept it except the _inst_id stuff, but I'd really like it
> to include a unit test (and a test which the current code wouldn't pass,
> of course :).  It would be helpful if you can also redo it against CVS.=20
> But the unit test is more important.  I am repeating that for emphasis
> ;)

Sir, yes, Sir ! ;)

Fran=E7ois