Saturday, May 30, 2009

Some Simple Scala Refactorings

 
I constantly go back and tinker with my old Scala code, especially in my CPU Simulator project. When I'm in the old code I notice something that I haven't really noticed before - the code is still good. In a previous life when I wrote Java code, I'd go back and look at old code and want to throw up. Scala is so concise that this just doesn't happen. Yes, I do find room for improvement here and there, but overall I'm still really happy with the code (there was an exception, when I didn't know anything about functional programming, and refactored from imperative to functional...but that doesn't count).

Anyway, I have a few refactorings that I wanted to mention.

Refactoring to Case Classes


I refactored all of my LogicGate classes to be case classes instead of regular classes. The reason? The code is just prettier. I'm not a huge fan of the new keyword in general, and I especially don't like it littering up my code.

Old Code:

class XorGate(inputA: PowerSource, inputB: PowerSource)
extends LogicGate {
val output: PowerSource =
new AndGate(
new OrGate(inputA, inputB),
new NandGate(inputA, inputB))
}

New Code:

case class XorGate(inputA: PowerSource, inputB: PowerSource)
extends LogicGate {
val output: PowerSource =
AndGate(OrGate(inputA, inputB), NandGate(inputA, inputB))
}

The difference is small, but I literally had hundreds of new calls littered throughout my code. Now, I could also mention the extra power that case classes give as well - nice toString, pattern matching, hash code and equals... I happened to not really be using most of those things in the CPU simulator, so I don't have an immediate example. Oh well. In my opinion/experience, favor case classes over regular classes.

This does violates encapsulation somewhat though. Previously, inputA and inputB weren't accessible from outside the class. I can get around that easily enough by adding private to my vals:

case class XorGate(private val inputA: PowerSource,
private val inputB: PowerSource)
extends LogicGate {
val output: PowerSource =
AndGate(OrGate(inputA, inputB), NandGate(inputA, inputB))
}

Now I'm not exposing those fields, I still have all the power mentioned above, and I don't have the pesky "new" statements hanging around.

Refactoring to Fewer Files


I noticed something a little unsettling. I had separate files LogicGate.scala, AndGate.scala, OrGate.scala, NandGate.scala NorGate.scala, XorGate.scala. One for each type of gate. Several files, and they were all very very small, less than 10 lines each, all with a common package statement, and similar imports.

So I tried something - putting them all into one file. This is something I normally do by default now. I'm not sure when I started putting lots of classes into one file and not spreading them out...Anyway, in my opinion the result was a lot better. Instead of 6 files roughly 5-10 lines long, I have one file less than 40 lines long. I got rid of the redundant package and import statements. But most importantly, now I can see everything there is to know about my logic gates on the screen at one time.


package com.joshcough.cpu.gates

import electric.{Relay, Inverter, Wire, PowerSource}

object LogicGate{
implicit def logicGateToPowerSource( lg: LogicGate ): PowerSource = lg.output
}

trait LogicGate{
val inputA: PowerSource
val inputB: PowerSource
val output: PowerSource
}

case class AndGate(val inputA: PowerSource,
val inputB: PowerSource) extends LogicGate {
val output: PowerSource = Relay(inputA, Relay(inputB))
}

case class NandGate(val inputA: PowerSource,
val inputB: PowerSource) extends LogicGate{
val output = new Wire
Inverter(inputA)-->output
Inverter(inputB)-->output
}

case class NorGate(val inputA: PowerSource,
val inputB: PowerSource) extends LogicGate{
val output: PowerSource = Inverter(inputB, Inverter(inputA))
}

case class OrGate(val inputA: PowerSource,
val inputB: PowerSource) extends LogicGate{
val output = new Wire
inputA-->output
inputB-->output
}

case class XorGate(val inputA: PowerSource,
val inputB: PowerSource) extends LogicGate {
val output: PowerSource =
AndGate(OrGate(inputA, inputB), NandGate(inputA, inputB))
}


I know some people who have said they will never do this, never having more than one class in a file. I think that's wrong. When you have several small classes, seeing everything at once overrules most arguments.

More


I probably should suck it up and turn my logic gates into actual functions at some point in the near future. That should provide some really interesting material on refactoring. Until then, cya.

4 comments:

  1. Case classes don't force you to expose representation that you don't want to. Add the "private" keyword to the parameters.

    case class XorGate(private val inputA: PowerSource, private val inputB: PowerSource)

    ReplyDelete
  2. Yes, I know. But thanks. I was tired and apparently my mind wasn't working, I can be a real idiot sometimes. I'll update right now.

    ReplyDelete
  3. The old code has "val" before each class parameter, so they were accessible outside the class in that version too.

    As for files vs classes, some languages are so verbose that even very simple classes take a lot of space visually. But, then again, you already knew that, right? :-)

    ReplyDelete