My Mimprint program (now also on github) was originally written in Java, then ported to Scala soon after I first started learning that language. As such, much of that original ported code was "Java written in Scala". As I have continued to internalize the Scala approach I have gone back and modified various parts of the program to make it cleaner.
In one part of the program I set up a collection of menu checkboxes to allow the user to enable or disable various features. As those features are enabled or disabled, the states of other screen components change; sometimes a component is enabled or disabled, sometimes a component is hidden or made visible.
My original Java-ish Scala code to do this looked something like this (with irrelevant parts omitted):
There were two things about this code that I didn't like:class ViewListGroup ... { ... private var singleComp:Component = _ private var mShowFileInfo:SCheckBoxMenuItem = _ private var mShowFileIcons:SCheckBoxMenuItem = _ private var mShowDirDates:SCheckBoxMenuItem = _ private var mShowSingleViewer:SCheckBoxMenuItem = _ def getComponent():Component = { ... singleComp = playViewSingle.getComponent() ... //Add our menu items mShowFileInfo = new SCheckBoxMenuItem( viewer,"menu.List.ShowFileInfo")( showFileInfo(mShowFileInfo.getState)) mShowFileInfo.setState(true) m.add(mShowFileInfo) mShowFileIcons = new SCheckBoxMenuItem( viewer,"menu.List.ShowFileIcons")( showFileIcons(mShowFileIcons.getState)) mShowFileIcons.setState(false) m.add(mShowFileIcons) mShowDirDates = new SCheckBoxMenuItem( viewer,"menu.List.ShowDirDates")( showDirDates(mShowDirDates.getState)) mShowDirDates.setState(playViewList.includeDirectoryDates) ... m.add(mShowDirDates) mShowSingleViewer = new SCheckBoxMenuItem( viewer,"menu.List.ShowSingleViewer")( showSingleViewer(mShowSingleViewer.getState)) mShowSingleViewer.setState(true) m.add(mShowSingleViewer) showSingleViewer(mShowSingleViewer.getState) //make sure window state is in sync with menu item state ... } ... def showFileInfo(b:Boolean) { playViewList.showFileInfo(b) mShowFileInfo.setState(b) mShowFileIcons.setEnabled(b) mShowDirDates.setEnabled(b) } def showFileIcons(b:Boolean) { playViewList.showFileIcons(b) playViewList.redisplayList() } def showDirDates(b:Boolean) { playViewList.includeDirectoryDates = b playViewList.redisplayList() } def showSingleViewer(b:Boolean) { singleComp.setVisible(b) singleComp.getParent.asInstanceOf[JSplitPane].resetToPreferredSizes() mShowSingleViewer.setState(b) playViewList.requestSelect } ... }
- Mutable instance variables using
var
, particularly since they were not really variable. These values were being assigned once, not at construction time, but had to be available to other methods. - The close binding between the different UI components, since the action method called by one component directly modified attributes of possibly a number of other components.
ActorPublisher
class, where each
subscriber is an Actor that accepts messages of the published object type,
but in this case I wanted a lighter weight implementation, since I knew
the subscriber actions would be quick.
Also, this being
Swing, the subscriber actions that update screen state
should run in the Swing
event thread, and the events being published are
also coming from the event thread, so the simple thing to do is to run
the subscriber actions directly from the publish method.
Writing a publish/subscribe handler in Scala is pretty easy, and for me it was even simpler, as I already had one. I grabbed my
ListenerManager
and modified it to use the publish/subscribe terminology.
I also added synchronization to make it multi-thread safe, although
for this app I don't really need it.
It now looks like this:
For each menu checkbox I would like to set up a publisher. In every case, I just need to publish whether that checkbox has just been enabled or disabled. I defined a simple case class hierarchy to represent thepackage net.jimmc.util /** Manage a subscriber list. * There are no guarantees on the order of subscribers in the list. * This code is a slightly modified version of ListenerManager * as published to my blog in April 2009. */ trait Publisher[E] { type S = (E) => Unit private var subscribers: List[S] = Nil private object lock //By using lock.synchronized rather than this.synchronized we reduce //the scope of our lock from the extending object (which might be //mixing us in with other classes) to just this trait. /** True if the subscriber is already in our list. */ def isSubscribed(subscriber:S) = { val subs = lock.synchronized { subscribers } subs.exists(_==subscriber) } /** Add a subscriber to our list if it is not already there. */ def subscribe(subscriber:S) = lock.synchronized { if (!isSubscribed(subscriber)) subscribers = subscriber :: subscribers } /** Remove a subscriber from our list. If not in the list, ignored. */ def unsubscribe(subscriber:S):Unit = lock.synchronized { subscribers = subscribers.filter(_!=subscriber) } /** Publish an event to all subscribers on the list. */ def publish(event:E) = { val subs = lock.synchronized { subscribers } subs.foreach(_.apply(event)) } }
Enabled
and Disabled
messages:
I then created a publisher class that uses that event type:sealed abstract class Abled case object Enabled extends Abled case object Disabled extends Abled
I want to easily publish theclass AbledPublisher extends Publisher[Abled]
Enabled
or Disabled
object based on
the current state of a checkbox, so I added an AbledPublisher
companion object with an apply
method to do that:
Conversely, upon receiving anobject AbledPublisher { object Abled { def apply(b:Boolean) = if (b) Enabled else Disabled } }
Abled
event
in a subscriber for a UI component I want to be able to
enable or disable that component.
I could use a match
statement with cases for
Enabled
and Disabled
,
but a simpler way is to
modify the Abled
case class hierarchy to
encode a boolean state value into the Abled
case object to allow
easy translation from an Abled
object back to a state:
Finally, I packaged up the case class hierarchy inside thesealed abstract class Abled { val state:Boolean } case object Enabled extends Abled { override val state = true } case object Disabled extends Abled { override val state = false }
AbledPublisher
object to control scoping.
The final AbledPublisher
file looks like this:
Given the abovepackage net.jimmc.util //For subscribers of things that turn on and off class AbledPublisher extends Publisher[AbledPublisher.Abled] // use "import AbledPublisher._" to pick up these definitions object AbledPublisher { sealed abstract class Abled { val state:Boolean } case object Enabled extends Abled { override val state = true } case object Disabled extends Abled { override val state = false } object Abled { def apply(b:Boolean) = if (b) Enabled else Disabled } }
AbledPublisher
class and object,
I modified my code so that the action method called by each menu checkbox
publishes an Enabled
or Disabled
event that
matches the new state of the checkbox, and for each place in the old
code where an action method called a state-changing method on another
component I set up that target component as a subscriber to the
appropriate publisher that, when it receives a published event,
takes appropriate action on itself.
With the above changes, and a slight change to my
SCheckBoxMenuItem
class
so that it passes itself to the action callback, the code now looks
like this:
The total number of lines of code in ViewListGroup is actually a bit more than before, but I find the code a little easier to understand because all of the code that acts on a UI component is now localized in one place in the source file. All of theimport net.jimmc.util.AbledPublisher import net.jimmc.util.AbledPublisher._ class ViewListGroup ... { vlg:ViewListGroup => ... private val showFileInfoPublisher = new AbledPublisher private val showSingleViewerPublisher = new AbledPublisher private val showDirectoriesPublisher = new AbledPublisher ... def getComponent():Component = { ... val singleComp = playViewSingle.getComponent() showSingleViewerPublisher.subscribe((ev)=> { singleComp.setVisible(ev.state) singleComp.getParent.asInstanceOf[JSplitPane].resetToPreferredSizes() }) ... //Add our menu items val mShowFileInfo = new SCheckBoxMenuItem( viewer,"menu.List.ShowFileInfo")((cb)=> showFileInfo(cb.getState)) mShowFileInfo.setState(true) showFileInfoPublisher.subscribe((ev)=> mShowFileInfo.setState(ev.state) ) m.add(mShowFileInfo) val mShowFileIcons = new SCheckBoxMenuItem( viewer,"menu.List.ShowFileIcons")((cb)=> showFileIcons(cb.getState)) mShowFileIcons.setState(false) showFileInfoPublisher.subscribe((ev)=> mShowFileIcons.setState(ev.state) ) m.add(mShowFileIcons) val mShowDirDates = new SCheckBoxMenuItem( viewer,"menu.List.ShowDirDates")((cb)=> showDirDates(cb.getState)) mShowDirDates.setState(playViewList.includeDirectoryDates) mShowDirDates.setVisible(includeDirectories) showFileInfoPublisher.subscribe((ev)=> mShowDirDates.setState(ev.state) ) showDirectoriesPublisher.subscribe((ev)=> mShowDirDates.setVisible(ev.state) ) m.add(mShowDirDates) val mShowSingleViewer:SCheckBoxMenuItem = new SCheckBoxMenuItem( viewer,"menu.List.ShowSingleViewer")((cb)=> showSingleViewer(cb.getState)) mShowSingleViewer.setState(true) showSingleViewerPublisher.subscribe((ev)=> mShowSingleViewer.setState(ev.state) ) m.add(mShowSingleViewer) showSingleViewer(mShowSingleViewer.getState) //make sure window state is in sync with menu item state ... } ... def showFileInfo(b:Boolean) { playViewList.showFileInfo(b) showFileInfoPublisher.publish(Abled(b)) } def showFileIcons(b:Boolean) { playViewList.showFileIcons(b) playViewList.redisplayList() } def showDirDates(b:Boolean) { playViewList.includeDirectoryDates = b playViewList.redisplayList() } def showSingleViewer(b:Boolean) { showSingleViewerPublisher.publish(Abled(b)) playViewList.requestSelect } ... }
var
s
that held pointers to those components are now gone,
replaced by a few val
s for the publishers.
The publishers use var
s
to maintain internal state, but that state is
simple and easily understood,
well encapsulated and multi-thread safe.
There is still more cleanup work to be done in Mimprint. For example, in the above code the checkbox action methods such as
showFileInfo
and showFileIcons
call methods on the
playViewList
object as well as publishing an
Abled
event.
Instead, I could set up playViewList
as a listener on
each of the published events, then make the menu checkbox actions directly
publish an event and get rid of the showXXX
methods.
I will leave that for another round of cleanup.
2 comments:
You might want to take a peek at scala.collection.mutable.Publisher. It is not thread safe; however, it would not be hard to fix that either by wrapping it or by using it in a thread safe manner.
Brian
nairb774: Thanks for the pointer to Scala's Publisher class, I should have looked in the standard library first. The problem with that Publisher is that the subscribe method takes an instance of a Subscriber rather than a closure, which makes it more of a hassle to use for simple in-line callbacks such as I wanted to do here. scala.swing.Publisher does take a closure, but that trait is not type-parameterized. So I will stick with my light-weight Publisher for this use.
Post a Comment