Code review comment for lp:~jpakkane/unity-scopes-api/simplelogging

Revision history for this message
Michi Henning (michihenning) wrote :

We have RuntimeImpl in the code as a handle to anything that the library needs. That gives us the ability to have multiple runtime instances in a process (we do this in the tests, for example), and it allows us to control the life time of each run time.

With a global logger variable, we can't do this anymore, and separate instances of the runtime in the same process will share the same logger.

I think it would be better to have RuntimeImpl own and initialise the logger. I believe that it's possible to get at the RuntimeImpl/Runtime instance from most places in the code. There may be places in value types where we would like to log something and can't get at the Runtime handle. I haven't checked this in detail yet, but I expect there won't be too many places in the code where this is the case. In most of them, we can probably make the logger available if we need it.

At any rate, I'm uncomfortable with having a global variable.

« Back to merge proposal