3
\$\begingroup\$

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)
  }

}
\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.