enthought/envisage — comment on commit a58e58b5fbaf99dd0e29874967743846b4ea6d78
File:  line 0
By mchilvers:

Just a couple of quick comments:-

- To help our Envisage applications scale plugins should only import the 'real' code when necessary. In this case you can put the import of EventManager into the '_evt_mgr_default' method and then specify a string ' encore.events.event_manager.EventManager' as the protocol when creating the service offer (which also gets around the smell in line 35 ;^)

- I don't think typing 'evt_mgr' instead of 'event_manager' is a win. A property of readable code is that you start to be able to guess names of classes/functions/methods/variables etc, and abbreviations are hard to guess. If OTOH, you simply map the class name into an attribute name then I can pretty much guess the names every time.

- I'm not a fan of tagging arbitrary attributes onto Python objects at runtime as it makes things tricky to debug. In this case if the notion of a singleton event manager is useful then put that code in the event manager package. If you have a module level attribute with a getter and setter you can do it without forcing the EventManager class to always be used as a singleton).