Code review comment for lp:~acsone-openerp/ocb-addons/7.0-bug-796570-amu

Thank you for your review,

You are perfectly right. I'll add those clarifications in the next days.

> Thanks for your efforts!
>
> I think what you did is perfectly fine, but the names and docstrings of the
> methods you introduce need improving: They pretend to return an id, while in
> fact they return a list of ids (which very often only has one element). So I'd
> rename the methods to get_tax_ids_or_default etc. and update the docstring
> accordingly. Sounds like nitpicking? I don't think so, for API functions it's
> very important that you get what you expect, and with a name phrased in
> singular, you expect a scalar, for a plural, you expect an iterable. And the
> fact that this doesn't apply to OpenERP's method/field names counts against
> their choice of names in the early days that we still have to live with now
> for legacy reasons, not for continuing doing this where it's not necessary.
>
> Another thing is that I doubt '_or_default' needs to be part of the name. That
> should rather be part of the docstring.
>
> Then if get_{supplier_,}taxes_or_default also accepted a list of ids, we could
> call it from a product's browse record, condensing the code a lot while making
> it more readable. Maybe add an assertion that the list can only contain one
> element.