OSGi-like infrastructure

I am a moderately new Scala developer working mostly by myself, so I don't have anyone to tell me what I'm doing wrong (except that the system mostly does work, so it can't be too awful). I would appreciate some feedback on how to make better use of Scala as well as any comments about obvious performance issues.

The class below is part of our utility library. It's part of our OSGi-like infrastructure. I didn't use OSGi because the web application is hosted on a Tomcat server but I liked many of the ideas behind it, so I've incorporated them into my own library.

```
package edu.stsci.efl.ml

import scala.collection.mutable.{HashSet, Stack, HashMap}
import scala.xml.Node

import java.util.Date

/**
* Provides a container for services that are being provided by various modules.
*
* The default context can be had from the ModuleLoader object.
*
* Services are accessed by the class of the trait that they implement. For example,
* logging services implement LoggerFactoryService so to get access the service, call
* getService(classOf[LoggerFactoryService]) to find the service that was defined in
* the ModuleLoader configuration file.
*/
class EFLContext(val name: String) {

private var modules: HashMap[String, EFLModule] = new HashMap[String, EFLModule]
private val shutdownList: Stack[EFLModule] = new Stack[EFLModule]
private val importList: Stack[EFLModule] = new Stack[EFLModule]
private val services = new HashSet[Object]

private var myDependencies: List[EFLContext] = null
private var myDependents: List[EFLContext] = null

/**
* Used in testing to verify that the correct number of modules have been loaded.
*/
def getModuleCount = modules.size

/**
* Use by the ModuleLoader to pass a module definition from the XML configuration file.
*/
def load(moduleDefinition: Node) {

val moduleName = (moduleDefinition \ "@name").toString()

val className = (moduleDefinition \ "@class").toString()

try {
v

Some words at beginning: I have never used OSGI, so I can't say if the tips below may all work in your context. If they do not, than keep them in the back of your head and use them when you can.

One of the most important rules is to avoid mutable state if possible. This will result in more type-safe and easier to read code. This is not always possible but most times it is.

Never use null if you don't have to use it. Use Option type instead or use empty collections.

private var myDependencies: List[EFLContext] = null
// =>
private var myDependencies: List[EFLContext] = Nil

Now you don't have to pattern match against null any more:

def addDependency(fLContext: EFLContext) {
  myDependencies match {
    case null => myDependencies = List[EFLContext](fLContext)
    case _ => myDependencies = myDependencies :+ fLContext
  }
  fLContext.addDependent(this)
}
// =>
def addDependency(fLContext: EFLContext) {
  myDependencies +:= flContext
  fLContext addDependent this
}

Here, never append to a List, this is inefficient (O(n)). Instead use prepending (O(1)). If you have to prepend elements, than use an IndexedSeq. If you want you can use operator notation.

In Scala everything returns a value, thus there is no need to mark a method that it returns a value:

def getModuleCount = modules.size
// =>
def moduleCount = modules.size

method -= is deprecated. Use filterNot instead.

Scala has some nice methods defined in Predef. One of them can check input params:

if (modules == null) throw new RuntimeException("Attempt to shutdown context that was already shutdown.")
// =>
require(modules.nonEmpty, "Attempt to shutdown context that was already shutdown.")

Don't traverse through the elements of a collection with a while-loop. Instead use higher-order functions (foreach instead of while-loop). Furthermore in Scala there is no need to explicitly use a Stack. A List is already a Stack (when you prepend elements what you should do)!

while (!importList.isEmpty) {
  val next = importList.pop()
  next.removeContext(this)
}
// =>
importList foreach { _ removeContext this }
importList = Nil

Pattern match Options:

if (opt.isDefined) ...
else opt.get ...

opt match {
  case None => ...
  case Some(o) => o ...
}

Use homogeneous types in collections and not Object if possible.

private val services = new HashSet[Object]
// =>
val services1 = new HashSet[Type1]
val servicesN = new HashSet[TypeN]

If you do so there is no need for casts any more like in your method findService. Of course you should choose better names for the variables than I did.

To easily come from null to Option you can use its apply method:

Option(null) // => None
Option(non_null_value) // => Some(non_null_value)

No need to use curly brackets in a case-block:

case _ => {
  a
  b
}
// =>
case _ =>
  a
  b

You can use Option in method resolveImport:

def resolveImport(data: Node) {
  val contextName = getAttribute(data, "context")
  val name = getAttribute(data, "name")
  val context = Option(ModuleLoader.getContext(contextName))
  val result = context flatMap { _.modules get name }
  result match {
    case None =>
      throw new IllegalArgumentException("Unable to resolve import of module '"+name+"' in context '"+contextName+"'.")  
    case Some(module) =>
      this addDependency module
      module addContext this
      importList push module
      modules += name -> module
  }
}

StackExchange Code Review Q#10030, answer score: 10