Hi all, Here is a part of the last commit in the point_of_sale module and I don't like that.
[code]
cr.execute(""" select id from account_journal
where auto_cash='True' and type='cash'
and id in (%s)""" %(','.join(map(lambda x: "'" + str(x) + "'",
j_ids))))
journal_ids = map(lambda x1: x1[0], cr.fetchall())
for journal in journal_obj.browse(cr, uid, journal_ids):
# do something
[/code]
Remarks:
========
1. We don't use the parameters of the execute method from Psycopg2 [BAD]
-> With these parameters we can avoid the sql injections
cr.execute(""" select id from account_journal
where auto_cash='True' and type='cash'
and id in (%s)""" %(','.join(map(lambda x: "'" + str(x) + "'",
j_ids))))
Why do you use the join and the map functions ? If you try to write this code,
we risk to get some errors during the run-time because we will introduce some
sql injections,
and it's bad :/
2. We use an SQL statement instead of the search method ? In this case, I don't
find the reason. [BAD]
3. We bypass the ir.rules mechanism and we risk to have a problem with the
security. [BAD]
Please try to be smart. We have guidelines [1], try to respect them.
We can rewrite the query with the search method and with this way, avoid the
sql query and the code is readable.
Example:
journal_obj = self.pool.get('account.journal')
journal_ids = journal_obj.search(cr, uid,
[('auto_cash', '=', True),
('type', '=', 'cash'),
('id', 'in', j_ids)])
for journal in journal_obj(cr, uid, journal_ids):
# do something
4. A commented code should be removed, we have a system to restore an old part
code (Bazaar) !
[1] Guidelines: http://doc.openerp.com/contribute/15_guidelines/
Quentin, Please, Could you ask to your developers to respect the guidelines.
I'm sorry but It's not very funny to read this kind of code !
Regards,
Stéphane
--
Stephane Wirtel - "As OpenERP is OpenSource, please feel free to contribute."
Quality/Release Manager
Technical Project Manager
OpenERP S.A.
Chaussee de Namur, 40
B-1367 Grand-Rosière
Tel: +32.81.81.37.00
Web: http://www.openerp.com
Planet: http://www.openerp.com/planet/
Blog: http://stephane-wirtel-at-tiny.blogspot.com
<<attachment: stw.vcf>>
_______________________________________________ Mailing list: https://launchpad.net/~openerp-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~openerp-dev More help : https://help.launchpad.net/ListHelp
