Comment 5 for bug 210177

On 2 Apr 2008, at 13:06 , Hermann Himmelbauer wrote:
> Ok, I added the above test with circular references, I raise a
> RuntimeError in this case (could not think of a better exception).

I suggest a ValueError instead.

> Moreover I now do a mapping.copy() and use parantheses for wrapping
> long
> lines.

Thanks, but the wrapping isn't exactly done how it's supposed to be.
This is the correct form:

   if (mapping is not None and
       Message in [type(m) for m in mapping.values()]):

I'm not very happy with the newest patch for other reasons, however.
You changed the signature of the translate() method. You can't do
that, it's governed by the ITranslationDomain interface. Public
interfaces like that must not change.

Stylistically, I have two things to nitpick about:

* Comparisons to None should be made with the 'is' operator (see PEP8)

* The way the 'to_translate' variable is used, a 'set' object would be
more suitable than a list.