sajty/ember — comment on commit c9ae91b6922d4857a5e0a10484ab9345e5916a90
File:  line 0
By erikhjortsberg:

Hi, before I can give better comments you must split up this commit in smaller commits. There are a lot of things here that concern different things than just the SDL->OIS change.
Somet things are formatting, some things are related to singletons, some things are clean ups. I will add some comments however here, but please split it into smaller commits and rebase.

Now, the first thing I want to mention is the usage of Singletons. You've done some changes in EmberOgre which unfortunately aren't in line with the design principles. In general, I want to avoid having singletons. It's an easy solution for a problem, and often something you add as a first implementation if you're now sure about the ownership. But they are messy and the encourage sloppy design, so I try to avoid them when possible. And I'm actively working to remove them. That's one of the reasons why there are places where they are stored as fields, even though they are singletons.
In addition, when using singletons in Ember the most correct way to handle them is to have them owned in one place. I.e. there should always be an owner instance which has a field which holds the singleton. The reason for this is to make ownership clear. If not, one can very easily come into a situation where it's not clear who, if any, is the owner of the lifecycle of a singleton instance.
The end result of this is that I must ask you to revert the singleton changes you've done to EmberOgre.

There are a couple of more things I want to comment on; I'll get back to you after work.