Magic Container

Name: Magic Container

Type: Development

Problem: Poor design and a non-OO approach has left your application code with lots of simple (primitive or fine-grained) variables. Method calls have lengthy parameter lists. Methods with reference parameters are poorly documented or understood.

Context: any procedural code running in an OO environment.

Forces: Passing many primitive parameters to a method call is known to be poor practice and it has been decided to refactor many fine-grained variables into something coarser. Bundling a success flag with several results into a method return object obviates the need for those icky reference params. There is insufficient time or experience to do the analysis and design to discover an appropriate class to own those values. Forget any understanding of encapsulation.

Supposed Solution: Maximize reuse by using a generic container object to hold all variables irrespective of type or context. You could really justify things to one's boss if you used a hashed collection (HashTable / Map) as you could then describe the values in the collection as quasi-attributes. Multiple return values combined with a method success code is now a cinch. Method signatures are now nice and compact:

 Map doIt(Map withThis)

You can even randomly chain the Collection that comes out of one method into the next without that pesky compiler ever complaining:

 Map foo = doThat(andThat(andWhyNotThat(new HashMap())));
 Map bar = doThat(andThat(andThisTimeThat(andWhyNotThat(foo))));

Resulting Context: The understanding of the origins and lifetimes of the objects in the collection is quickly lost. Finding what objects are in the Magic Container, their type, what key an object is stored under, are all a function of good fortune and not doing anything drastic with the existing code. The collection quickly bloats as no-one dares take anything out of the collection in case it might be used elsewhere. In truly horrible cases the same key references different objects in different contexts and the same object (or perhaps a clone) can be in the container several times over with different names.

Design Rationale: I'm doing reuse.. and encapsulation.. I must be good...

Appropriate Usage: not much.

Synonyms: BucketOfVomit

Other Occurrences: (for the J2EE wallahs) The HTTPServletRequest can contain any amount of decontextualized gunk. Stick in a few attributes and pass it wholesale into all your method calls. Chaining servlets gives you *wonderful* flexibility - just sling a few more attributes on the request and forward it on. In no time, you will have absolutely no idea where anything came from or where it might be used.

ReFactored solution: Make a class/struct with the parameters to pass in as data members - a ParameterObject. The default constructor can be used to assign meaningful default parameters. Allows you to simulate named parameters with some ease (though languages that don't allow convenient aggregate initializers might find this approach painful). In a language with StaticTyping; the compiler will make sure that the appropriate ParameterObject is passed in.

-- AndyGray?

You can make this even worse by nesting maps.

As to the refactored solution above, it's not ideal. Rather than using some ParameterObject that works only for the method, try to identify clusters of parameters that that make sense and turn those into objects that might be useful across the codebase. In other words - a parameter object is simply a mechanical fix, although it is somewhat better because you have some compile-time safety. Try to engineer a proper solution.

If it's not doable, at least put a "verifyme" method in the parameter object which is called on entry into the method. Its job is to throw an IllegalStateException if the object wasn't set up right. You can also move control logic into the parameter object, to determine what combinations of parameters are set (although this is horrible). Alternatively, you can move the method in question into the parameter object, and if different combinations of parameters make different things happen, create subclasses with polymorphic implementations.

thus mungName(pers1, pers2, pers3, org1, org2, foo1, foo2, foo3)

becomes

 abstract class nameMunger {
   foo1, foo2, foo3;
   abstract mungit()
 }

 class personnamemunger extends namemunger {
   pers1, pers2;
   mungIt() { ... }
 }

 class orgnamemunger extends namemunger {
   org1, org2;
   mungIt() { ... }
 }

with appropriate constructors.

-- PaulMurray

This is a form of StampCoupling (see also CouplingAndCohesion), and as such, may be an improvement over what had gone before, but is a less than desirable solution.

KeywordParameterPassing is also a solution to long positional parameter lists. This should be included with the above solutions. Associative arrays can also be used.

The only use I have seen for MagicContainer is remoting. This simplifies the remoting code enormously in comparison to the cost of implementing the calls. It also really helps if the MagicContainer itself validates the objects being added can be transferred through the remoting barrier (by type if possible). --JoshuaHudson