Code review comment for lp:~therp-nl/account-financial-tools/6.1-account-purchase-default

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Stefan, I will take your performance tests in mind when reading single character (what about float and integer, I guess will be about the same?) fields in the future.

Alexandre: thanks for your comments, with one exception I will use them to update the code.

The exception is on the need to test for partner_obj... I never want to make assumptions in my code on the correctness of calling functions. So indeed, I think it is necesarry to handle the case of a non existing partner_id. Or the case that should be even more rare that a valid partner_id will still not result in a valid partner object. For instance when the connection with the database has been lost.

The only thing is what to do about it.

In most cases I would prefer to have an assertion error. partner_obj should exist. If it does not, it is most likely due to a programmer error. In this case, retrieving a default value for a column that a user can change anyway, I don't want to blow up - confront the user with an exception - when something goes wrong.

« Back to merge proposal