Dependency injection instead of static singleton WorldWind

[stolsvik]

I added a JIRA WWJ-38, but was asked to put the question up here at the forums.

Here's a synopsis:

I'd like if the WorldWind "runtime" was assembled using IoC (Dependency Injection), instead of using the static singleton WorldWind which again uses the AVKey constants interface for configuration from a file (properties file-based configuration should be an option, not the option).

In addition, there seems to be no way to "turn off" WorldWind, or more specifically, "reboot" it. The problem now is the thread pools - there isn't any way to shut them down. The interfaces whose instances may create "persistent resources" (e.g. threadpools) should have a destroy() or close()-method, OR those such resources they use should be injected (so that they may be shut down from the outside).

Given that this aims to be a SDK, and as such should be really modular and "composable", I suggest that pretty much all classes which the base of WorldWind consist of should be interface-based (which they are, or are going to be, judging from the comments), AND that the corresponding classes has constructor-based dependency injection, instead of as now doing a static access to the WorldWind singleton for all their dependencies (The methods memoryCache(), dataFileCache(), retrievalService(), threadTaskService(), and the base createComponent(...)). This would enable different stack-ups, and should also make unit testing way easier.

Also, the default classes (the ones shipped with WorldWind) should not have private members (nor final methods), so that it would be easy to extend them.

Guice is a new, really cool IoC framework, and Spring is the obvious good'ol alternative, which both could be used to fire up the demo.

However, again, as stated in the JIRA: I'm pretty new to this codebase (I found this fantastic piece of code today! ), so I might still be missing fundamental elements that makes this complaint nonsense.

[tag]

WWJ defines interfaces for most of its major data structures so that the default implementations can be replaced. Most interfaces and many classes also implement the AVList interface, which enables specification of arbitrary information. The concrete Layer classes use this pre-construction, and others use it post-construction. An example of the former is:

public GeoCover1990()
{
super(makeLevels());
this.setUseTransparentTextures(true);
}

private static LevelSet makeLevels()
{
AVList params = new AVListImpl();

params.setValue(AVKey.TILE_WIDTH, 512);
params.setValue(AVKey.TILE_HEIGHT, 512);
params.setValue(AVKey.CACHE_NAME, "Earth/Geocover 2000");
params.setValue(AVKey.SERVICE, "http://worldwind25.arc.nasa.gov/TestWebApp/WebForm1.aspx");
params.setValue(AVKey.DATASET_NAME, "es_1990jpg");
params.setValue(AVKey.FORMAT_SUFFIX, ".dds");
params.setValue(AVKey.NUM_LEVELS, 5);
params.setValue(AVKey.NUM_EMPTY_LEVELS, 4);
params.setValue(AVKey.LEVEL_ZERO_TILE_DE LTA, new LatLon(Angle.fromDegrees(36d), Angle.fromDegrees(36d)));
params.setValue(AVKey.SECTOR, Sector.FULL_SPHERE);

return new LevelSet(params);
}

At some point some piece of code has to read configuration information from a location external to the application, such as a file. We don't expect applications to load WWJ configuration info and pass it to WWJ. worldwind.properties contains that info. It identifies not only config parameters but the names of the top-level classes to instantiate. There are only two that are fixed: WorldWind and Configuration. The former holds objects that are global to the system because the activities they implement must be coordinated. Configuration can be modified both statically, via worldwind.properties, and programmatically via setValue(). The rest -- retrieval service, task manager, scene controller, etc. -- can be anything you want to substitute for the defaults, as long as they implement the interface contract.

What is the real-world scenario in which shutting down the retrieval service and task service thread pools is necessary or useful? When there's no activity these objects are idle and can be empty. The retrieval service is also fully replaceable; the task service should be but isn't currently.

While we haven't achieved it consistently, our goal is to design for extension or preclude it. (See Effective Java by Bloch if you want background.) Not every class is meant to be arbitrarily extensible, typically because there is logic they must execute to implement their interface contract. The code isn't just doing simple stuff. There's a lot of complexity and algorithmic detail that no sub-class is going to get right and could easily destroy. That's what WWJ is all about, implementing the hard stuff. If the contract can't be ensured under extension then the class is useless. Arbitrary extensibility is essentially no extensibility.

We're happy and eager to consider any changes that enable real-world scenarios that fit with the APIs objectives. But we need to know what can't be done but could be done if only. I mean specific things, not simply "IoC," which is merely one relatively narrow technique of many to affect logic, and inappropriate in many cases. I should say, too, that requests to do something because it's "the way" are lost on me. I've seen too many "the ways" and the current crop is no different. Most are fashion. The world is littered with dead frameworks. The only "way" that I aspire to is common sense.

-- Thanks. I'm truly eager to hear those real-world examples. I hope you'll take the time to write them.

[stolsvik]

First of all: "the way" wasn't meant to be any "just because". It's more because there is a general consensus in the community by this time that the dependency injection principle is a much better approach to application assemby than any previous idea.

Have you read Martin Fowler's article which explains this pattern, and the service locator pattern, in length? Rod Johnson's book "Expert One-on-One Java EE Design and Development" also delve into this, and is basically the foundation for the Spring Framework. Joshua's book is a bit dated, being from 2001, and doesn't mention IoC or DI much. This might be because the dependency injection pattern really hadn't settled as a Java idea quite yet (I've understood that the first time it came up was in 2002, then popularized more in 2004 (Fowler), and is now all the rage, although the first mentions might apparently have been all the way back in 1980).

The general idea with dependency injection is that instead of each element of the code (typically each class) "going out and fetching" their dependencies, which are those other pieces of code that it has to collaborate with, they are instead given their dependencies.

This is a exceptionally small change in the code. Basically, classes' constructors takes their dependencies as arguments or the classes have setters for their dependencies, instead of them either instantiating the stuff themselves or going to a central place to get it (In addition, one also have to program to interfaces, not concretes, but that's not of issue here). However, this small change reaps huge benefits in terms of application setup (piecing together the different elements of an application), application extendability, code reuse, differing setups, and last but not least testability.

The reason for this is that instead of a central location for "service fetching" from a central, hardcoded place, typically called a service locator (as the the class WorldWind is an absolutely perfect example of), each component is utterly oblivious to it's surroundings, since it will be handed its collaborating partners from the outside. When this principle goes together with the use of interfaces, the benefits above comes as a direct consequence. In particular, one may fire up just one of the components, feeding it "dummy" or "simplified" implementations of the services it expects, typically when testing the component, or one may feed them entirely different implementations, typically when embedding the code within other code.

Do also note that "IoC" has nothing what so ever to do with a specific framework. This particular idiom cannot ever become a "dead framework that litters the world", since there is no actual framework. The sole principle is that instead of the code itself fetching its dependencies, they are being given to the code. I'll repeat this, and why WorldWind should use it, and why WorldWind won't have to change its basic functionality at all, some more times..

From a .NET guy's blog:
" ... here are the facts as I see them:

1. Dependency Injection is an important pattern for creating classes that are easier to unit test in isolation
2. Promotes loose coupling between classes and subsystems
3. Adds potential flexibility to a codebase for future changes
4. Can enable better code reuse
5. The implementation is simple and does *not* require a fancy DI tool

The PicoContainer team even has silly tee shirts printed up that say “I expected a paradigm shift, but all I got was a lousy constructor function.” DI is certainly a case where a minimum of effort supplies quite a bit of benefit. Don’t blow it off just because it seems trivial. "

An excerpt from Wikipedia's Inversion of Control article:
" Inversion of control relates to the way in which an object obtains references to its dependencies. The common approach of obtaining these dependencies from inside the object, either as singletons or by using some other lookup mechanism, has the disadvantage that the object then has an explicit dependence on that lookup mechanism, and any caller of the object is dependent on not only the object but that object's environment of dependencies as well. With inversion of control the object is passed its dependencies through constructor arguments or after construction through setter methods, meaning it may remain unaware of any external environment. This approach is also called 'dependency injection' since the dependences of an object are 'injected' into it. "

From Martin Fowler's article, this is an applicable excerpt (about a fictional "Move Lister" application):
" ... But what happens when my friends are overwhelmed by a desire for this wonderful functionality and would like a copy of my program? If they also store their movie listings in a colon delimited text file called "movies1.txt" then everything is wonderful. If they have a different name for their movies file, then it's easy to put the name of the file in a properties file. But what if they have a completely different form of storing their movie listing: a SQL database, an XML file, a web service, or just another format of text file? "

Fowler on service locators vs. DI " .. The difference [between service locator and Dependency Injection] comes if the [code at hand] is a component that I'm providing to an application that other people are writing. In this case I don't know much about the APIs of the service locators that my customers are going to use. Each customer might have their own incompatible service locators. I can get around around some of this by using the segregated interface. Each customer can write an adapter that matches my interface to their locator, but in any case I still need to see the first locator to lookup my specific interface. And once the adapter appears then the simplicity of the direct connection to a locator is beginning to slip. "

Okay, the WorldWind code have a properties file that actually define the concrete classes, so then this is all dandy? The problem is that this makes it rather impossible for me to use the IoC pattern if I want to: Using the properties file, I may change which implementations are instantiated. However, since each class needs an no-args constructor, and I don't have any influence on that particular code path, I cannot inject any dependencies there. And since the WorldWind class acts as the sole instantiator of classes, I cannot even stash the objects up with any other dependencies using setters, since I have no injection point in the code path. WorldWind forces me to use the static singleton pattern for the entire WorldWind component and its surroundings.


I further disagree with the notion that "At some point some piece of code has to read configuration information from a location external to the application, such as a file."

The main point here is that a piece of code doesn't have to read some configuration. Maybe the application as a whole has to read some configuration, but the individual pieces of code does not - they may simply be told how they should interact. This is way easier if I may give them the means for communication, instead of themselves going out to get it. This is because the latter approach requires some form of fixed point - namely a static singleton as the class WorldWind, while the former leaves this completely open: I may piece it together hardcoded, which for very many use cases is perfectly okay, or I may use some configuration stuff where I define each type's implementation using a properties file (!), or I may use any of the multitude of IoC containers out there (... or I may use WorldWind's own properties file: read on...)

This piece of code is supposed to be an embeddable component, as I've understood it. This means that this code will in many cases not be the main part of the system, but merely a part. This is the case in my use-case: I'm making a system where users should be able to enter GPS coordinates, and giving them the opportunity to literally select this from a fantastic interactive zoomable globe is really absolutely amazing. However, it will not be the main part of the application - merely a very nice, but actually rather seldomly used, element of it.

My application will obviously be handling the setup/assembly of my code (and in my particular case, I use the Google Guice framework, although there are several other well known systems, particularly Spring Framework and Pico Container). The WorldWind SDK at that point becomes a "big lump" that I cannot easily control - I am forced to use the particular idiom that you folks have chosen, not being able to do the piecing together myself, and as you tell me, I cannot even control its resource usage myself.

Martin Fowler on configration files: " My advice here is to always provide a way to do all configuration easily with a programmatic interface, and then treat a separate configuration file as an optional feature. You can easily build configuration file handling to use the programmatic interface. If you are writing a component you then leave it up to your user whether to use the programmatic interface, your configuration file format, or to write their own custom configuration file format and tie it into the programmatic interface "


At this point it may be worth pointing clearly out that changing of the WorldWind Java SDK to be "IoC compatible" is a extremely simple and easy task, and if it is so desired, the properties system emplyed may still be shipped exactly as it is now: Instead of having the static singleton WorldWind service locator, you'd merely have an _outside_ class that reads the properties file, and then instantiate each component that is needed based on the config in this file, and then stick these code pieces togheter. This class may be called WorldWind if that is desired!

This sticking together part is the exact point of each of the Spring IoC container, the Guice framework, and Pico Container - they do this setup using some sort of configuration file (or in Guice's case, using @annotations, and some java code (or Groovy, or Javascript ..)). However, if the particular properties file that WorldWind ships with is "holy", then there is absolutely no problem: just make this couple-of-hundreds-lines class oneself - it would actually look strikingly similar to what the WorldWind class does now. Or one may use Spring but change the configuration provider from the xml-stuff they use now, to one that reads properties files - I might think they actually have one of those in the code already.

When it comes to the need of being able to shut down the WorldWind: I believe I gave an example - the "reboot" example, and detailed which several benefits I see in this. The main point is that WorldWind won't necessarily be "The Main Aspect" of some application - it might simply be a part of some application. Here's an example, that will have to serve as an example for the general principle just stated: I have a big application where I occasionally need to let the user pick a location on the globe. At that point I'd like to fire up WorldWind, and let the user do the picking. Afterwards, since this isn't the central element, I'd like to get rid of the entire thing, freeing up heaps of resources: code-memory and memory (the classes, and the "globe", and the tiles cache, should be garbage collected, and the JVM heap will then shrink), and all its threads. Please don't counter this with the "It'd be swapped to disk"-argument - Java's memory system doesn't let this happen properly, since it constantly changes around on the memory using the copy collector and mark-and-sweep algorithms - and using such an amount for a completely inert piece of a system is rather non-elegant at any rate. What if this is just one of many such elements that think that they specifically are the "sole part" of the application - 500 MB here, and 500 MB there .. And what if the user actually wants to start some other application too?

Basically, I want my application to have complete control over what resources it allocates, and uses, during the whole application's life cycle.


The whole argument boils down to this: The code changes needed are really very small. The WorldWind class, and the properties file may still be a part of the system, letting everything work as it does now. These small changes would however enable every single user of the WorldWind Java SDK to choose to control the resources and choose to use IoC, or they may use the embedded system just as now: the WorldWind class and its properties file.

I was about to do these changes myself to the code, but stopped and filed the bug, since many others also would benefit from this, and that it would be very annoying to keep an actual fork of the project, when this probably was a good idea for everybody.

I also stated in the JIRA that I'd be willing to do these small changes and contribute them back, if this is an option.

Please also note that I'm not talking about every nitty gritty detail of the system - obviously these are already hardcoded in the classes - one doesn't let all Map implementations and We're only talking about the "big pieces" that the WorldWind class now handles.

Some more readings:
The term is humorously called The hollywood principle.
Wikipedia Dependency Injection.
Apache's HiveMind's page about IoC.
Apache's Excalibur's page about IoC.
A random blog entry that goes in some length about the paradigm.
The rather new JSR 250 defines a @Resource annotation for resource injection, described further in this sun article.

PS: Name one single place where "IoC isn't appropriate", given that the sole difference between a "Old-skool" dependent class and a "DI class" is whether it fetches, or is given (by means of constructor arguments or setters), its dependencies?

[stolsvik]

Since you mentioned these classes:

AVKey: check up on Effective Java Item 17. Although this interface isn't implemented anywhere, Joshua states that such constants should go in an uninstantiable class (a final class with a private constructor).

AVList: is this "general key/value container" pattern really that useful? I know elements of Swing rely on it, and I've seen it in other libraries, but it always leave one confused: where does which key work?

Why not let these keys instead be setters defined in an interface that other interfaces extend, and classes implement? And if some classes don't use all the AVKeys, well, then you are directly at my point: it is difficult for an user to know. If the AVKeys instead was divided into several sets of setters on different interfaces, then other interface or class could extend and implement the actual setters-sets that it use, and not implement the other. This would be really perfect for user using an IDE, it would be better for the JavaDoc, and it would also be better for speed (assuming that most key/values are set in some Map implementation, and not in actual instance variables).

[tag]

Thanks for taking the time to write all that. As much as I wish I had, I have no time to comment. Please just post or send (tom@tomgaskins.com) your proposed changes. We'll seriously consider them. We of course want to produce the best we can, and we appreciate thoughtful perspectives.

I will say that there are many competing objectives for WWJ. Some want virtually a one-line instantiation for the complete functionality. Others like yourself want absolute control and granular configurability. To balance objectives we have to apply what we know about this genre of technology and its usage, and neither is typical.

Thanks.

[stolsvik]

Several other projects have this balancing act situation, but solve it by having multiple entrances, e.g. one where you may say new StandardServer(), and get a good "director's cut", or you may piece it together yourself, going down the total control road.

Which code should I code against if I were to submit a patch?

[tag]

Please code against the code you have. A new release is coming within days, and we'd like to see what you propose in case we can get it in for that release. Thanks.

[stolsvik]

Isn't there a SVN or CVS somewhere? I find it a bit weird to code against code that definitely is stale - a patch against the 0.2.0 code cannot possibly be merged against what you have now?

[tag]

There's no svn or cvs we can give public access to. You said the changes are extremely simple and easy. Can't you just describe them and give a few examples?

[stolsvik]

The changes are very simple - there is is just quite a bunch of them! (This because there are quite a bunch of static accesses to Configuration and WorldWind).

The largest one is the Configuration static, since 16 classes refer to it.

The "isWindows" methods are okay to be in a static, so NitfsMessage's usages doesn't count. It would however probably be an idea to let the classes know beforehand which OS it is on, and do these "switch"-statements before injection.

The next is obviously WorldWind, which have the component-getters.

I've started coding now, but it feels a bit useless if this is just supposed to be "PoC", and then you'll (have to) do it again if you think it is okay.

There are several main rounds, with subrounds..

A) The DI-pattern change
B) The shutdown methods (*very* small issue)
C) The logger-situation (621 instances!), ref JIRA WWJ-37. But the static accessor here doesn't really break DI much - it may simply be viewed as a (inefficient) factory pattern, instead of concrete instantiation. It should however be fixed at some point.

I would do the DI-pattern change as follows, remember that I haven't made the code, so you might know of shortcuts!

0) Make interface of the ThreadedTaskService (as the comment also state)
1) Get rid of the WorldWind.
By appending "_not" on the two create* methods, and the four accessors to the major parts mentioned above, I get 61 errors, spread over just 13 classes.
I will have to track the code-path back to the "initial instantiation", which are the instantiation of WorldWindowGLCanvas in the BasicDemo.

Basically, the all AVKeys that end in "CLASS_NAME" should be concretely instantiated (apparently 11 classes), and given to the WorldWindowGLCanvas. The Layers to BasicModel would be List<Layer>.

A solution could be to change the WorldWind to an interface with some getters on (The classes at hand), and then instantiate some WorldWindImpl, and simply let that be a "carrier" for these component instances. This is however not "as pure" as giving them separately.

Indeed, I have this hunch that some elements of the code are tighter bound than others, and it will probably be preferable to first instantiate these and bind them together ("inject them"!), and then given these aggregate instances to WorldWindowGLCanvas.

The point is that this process must be somewhat iterative for my part, since I don't have the full class connection diagram in my head just yet!

2) Get rid of the Configuration - although we've basically done away with much of it already. The rest can be put in a configuration interface, with getters, or in a key/value Map, or for this initial round simply be hardcoded - the point is that also these details shall *preferably* be directly injected.

After this, it would be time to again build up the AVKey system, so that one may configure the entire WorldWind using the existing properties-file. If it is very undesirable to use an existing solution like Guice, PicoContainer or Spring, a simple dedicated dependency injector container will be coded.

Nobody that doesn't care will know that anything happened, but we that do care will be very happy!

If you'd like me to do all these changes, I'll do them. But if you already know at this point that they will never actually be merged back into the code, then I don't quite see the point. Maybe it will be better to wait for your new code-drop, and apply the changes to that instead?

PS: Are you planning to go "community open source" and give read access to the repository at some point? It feels a bit better to contribute code against "live code" than against stale tarballs.