Functional SaveObject in Scala

Question

I came across Java like Scala code that I am trying to refactor to make it functional and immutable. The first obvious flaw is that it's violating thread safety by using mutable public class level collections.

Those methods (insText, insInt, etc) look unnecessary and can be removed with higher order function (where function takes in dd => Int, dd => String type of thing). Class DDType is wrong with all mutable variables. Should I change class level mutable sets to private immutable sets?

Do you agree with those changes and overall refactoring approach?

class SaveObject extends Update with QueryFormatter {
 val keys = mutable.Set[(DomainDictionary, Int )]()
 val inserts = mutable.Set[(DomainDictionary, String)]()
 implicit def toInt(value:String) = value.toInt
 implicit def toDouble(value:String) = augmentString( value ).toDouble
 implicit def toBool(value:String) = value.toBoolean
 implicit def toTs(value:String) = {
 val date:Date = dateFormat.parse(value)
 new Timestamp(date.getTime)
}
implicit def toDate(value:String):Date = {
 dateFormat.parse(value)
}
def add(dd:DomainDictionary, value:String) = {
// check if its a key
getType(dd) match {
 case x if x.isKey => id(dd, value)
 case x if x.isText => insText(dd, value)
 case x if x.isInt => insInt(dd, value)
 case x if x.isDouble => insDouble(dd, value)
 case x if x.isBool => insBool(dd, value)
 case x if x.isTs => insTs(dd, value)
 case x if x.isDate => insDate(dd, toDate(value))
 case _ =>
 val msg = "Could not convert type for dd %s and value %s".format(dd.id, value)
 throw new Exception(msg)
 }
}
class DDType(dd:DomainDictionary) {
var isKey = false
 var isText = false
 var isInt = false
 var isDouble = false
 var isBool = false
 var isTs = false
 var isDate = false
 }
 def getType(dd:DomainDictionary):DDType = {
 val theType = new DDType(dd)
 val ns = dd.namespace
 val isKey = namespaceMatcher(ns,
 (ns,table,key)=>false.toString,
 (ns,table,key)=>false.toString,
 (ns,table,key)=>if(key == QueryPathfinder.ID) true.toString else false.toString
)
 if(toBool(isKey)){
 theType.isKey = true
 }
 else {
 val ddType = dd.domainDictionaryType
 ddType match {
 case "text" => theType.isText = true
 case "numeric" => theType.isInt = true
 case "alphanumeric" => theType.isText = true
 case "date" => theType.isDate = true
 case "timestamp" => theType.isTs = true
 case "boolean" => theType.isBool = true
 case "currency" => theType.isInt = true
 case _ => {} //this is wrong
 }
 }
theType
}
def id(dd:DomainDictionary, key:Int){
 keys += Tuple2(dd, key)
}
def insText(dd:DomainDictionary, value:String) = {
 inserts += Tuple2(dd,value)
}
def insInt(dd:DomainDictionary, value:Int) = {
 inserts += Tuple2(dd,value.toString)
}
def insDouble(dd:DomainDictionary, value:Double) = {
 inserts += Tuple2(dd,value.toString)
}
def insTs(dd:DomainDictionary, value:Timestamp) = {
 val date = new Date(value.getTime)
 inserts += Tuple2(dd,dateFormat.format(date))
 }
 def insDate(dd:DomainDictionary, value:Date) = {
 inserts += Tuple2(dd,dateFormat.format(value))
 }
 def insBool(dd:DomainDictionary, value:Boolean) = {
 inserts += Tuple2(dd,value.toString)
 }
}

Answer 1

You are mixing a repository with a classifier. The classifier is not doing anything except separating out keys from non-keys. The repository doesn't do anything with what it holds. You convert strings to a particular type and then store them back as strings, without gaining any real value along the way.

DDType doesn't seem to do anything except act as a classifier. And it doesn't even do that, as it only translates the string values "text", "numeric", etc, into predicates which are then discarded at the end of the classification.

Without seeing the use case, I can only presume that add(dd) is called from time to time and the sets are updated, either into keys or into inserts.

1. keys and inserts can be variables that hold immutable Sets. That will improve thread safety a smidgen.

2. Remove the redundant classifer:

   def add(dd:DomainDictionary, value:String) = {
    dd.domainDictionaryType match {
    case "numeric" => keys = keys + (dd, value.toInt)
    case _ => inserts = inserts + (dd, value)
    }
   }
   

3. For full thread safety, use a Repository Pattern, or use Actors.