Return to Answer

• It appears the entire listing is in one file. Maybe you did that just to submit it for review, but it would be easier to digest in separate files, each with their own imports. The way it was split up into Worker, Dispatcher, etc., would be a good way to split it into separate files as well.

• There are number of unused imports, this is cruft.

• The TimedActor trait is not actually used anywhere, so it doesn't add value to the example.

• I would use parentheses after sender(), iterator.next(), shutdown() and preStart() since these are more than just a property of the object. See Programming in Scala 10.3 Defining parameterless methods.

• The curly brackets are not necessary after case. I personally prefer to leave them out where they are not necessary.

• The reduceLeft call can be simplified:

  From

    (1 to 2000000).reduceLeft {
       _ + _
    }
  

  To

    (1 to 2000000).sum
  

• Unnecessary parentheses in a case filter:

      case x if ((0 to 500) contains x) ⇒ false
  

  can be written as

      case x if (0 to 500) contains x ⇒ false
  

• Simplify

    !workDone.valuesIterator.exists(_ == None)
  

  to

    !workDone.valuesIterator.contains(None)
  

• The following code

      val result = new WorkService doWork (workload)
  

  might be nicer written as

      val result = new WorkService.doWork(workload)
  

  or

      val result = new WorkService doWork workload
  

  (Personally I prefer the former).

• It appears the entire listing is in one file. Maybe you did that just to submit it for review, but it would be easier to digest in separate files, each with their own imports. The way it was split up into Worker, Dispatcher, etc., would be a good way to split it into separate files as well.

• There are number of unused imports, this is cruft.

• The TimedActor trait is not actually used anywhere, so it doesn't add value to the example.

• I would use parentheses after shutdown() and preStart() since these methods potentially have side effects. See Programming in Scala 10.3 Defining parameterless methods.

• The curly brackets are not necessary after case. I personally prefer to leave them out where they are not necessary.

• The reduceLeft call can be simplified:

  From

    (1 to 2000000).reduceLeft {
       _ + _
    }
  

  To

    (1 to 2000000).sum
  

• Unnecessary parentheses in a case filter:

      case x if ((0 to 500) contains x) ⇒ false
  

  can be written as

      case x if (0 to 500) contains x ⇒ false
  

• Simplify

    !workDone.valuesIterator.exists(_ == None)
  

  to

    !workDone.valuesIterator.contains(None)
  

• The following code

      val result = new WorkService doWork (workload)
  

  might be nicer written as

      val result = new WorkService.doWork(workload)
  

  or

      val result = new WorkService doWork workload
  

  (Personally I prefer the former).

* With credit given to the inspections in the IDEA Scala Plugin for automatically discovering many of these issues

* With credit given to the inspections in the Scala Plugin for IntelliJ for automatically discovering many of these issues

You said you were interested in "any feedback", so I will also add some suggestions on how you might make the code (even) cleaner. Some of this might be personal preference, so feel free to disagree:

Aside from a functional review, and you said you were interested in "any feedback", I will also add some suggestions on how you might make the code (even) cleaner. Some of this might be personal preference, so feel free to disagree:*

* With credit given to the inspections in the IDEA Scala Plugin for automatically discovering many of these issues