1

We are building some sync functionality using two-way json requests and this algorithm. All good and we have it running in prototype mode. Now I am trying to genericise the code, as we will be synching for several tables in the app. It would be cool to be able to define a class as "extends Synchable" and get the additional attributes and sync processing methods with a few specialisations/overrides. I have got this far:

abstract class Synchable [T<:Synchable[T]] (val ruid: String, val lastSyncTime: String, val isDeleted:Int) {
  def contentEquals(Target: T): Boolean
  def updateWith(target: T) 
  def insert
  def selectSince(clientLastSyncTime: String): List[T]
  def findByRuid(ruid: String): Option[T]

  implicit val validator: Reads[T]

  def process(clientLastSyncTime: String, updateRowList: List[JsObject]) = {
    for (syncRow <- updateRowList) {
      val validatedSyncRow = syncRow.validate[Synchable]
      validatedSyncRow.fold(
        valid = { result => // valid row
          findByRuid(result.ruid) match { //- do we know about it?
            case Some(knownRow) => knownRow.updateWith(result)
            case None => result.insert
          }
        }... invalid, etc

I am new to Scala and know I am probably missing things - WIP!

Any pointers or suggestions on this approach would be much appreciated.

Community
  • 1
  • 1
wwkudu
  • 2,778
  • 3
  • 28
  • 41

1 Answers1

0

Some quick ones:

Those _ parameters you pass in and then immediately assign to vals: why not do it in one hit? e.g.

abstract class Synchable( val ruid: String = "", val lastSyncTime: String = "",  val isDeleted: Int = 0) {

which saves you a line and is clearer in intent as well I think.

I'm not sure about your defaulting of Strings to "" - unless there's a good reason (and there often is), I think using something like ruid:Option[String] = None is more explicit and lets you do all sorts of nice monad-y things like fold, map, flatMap etc.

Looking pretty cool otherwise - the only other thing you might want to do is strengthen the typing with a bit of this.type magic so you'll prevent incorrect usage at compile-time. With your current abstract class, nothing prevents me from doing:

class SynchableCat extends Synchable { ... }
class SynchableDog extends Synchable { ... }

val cat = new SynchableCat
val dog = new SynchableDog

cat.updateWith(dog) // This won't end well

But if you just change your abstract method signatures to things like this:

def updateWith(target: this.type)

Then the change ripples down through the subclasses, narrowing down the types, and the compiler will omit a (relatively clear) error if I try the above update operation.

millhouse
  • 9,817
  • 4
  • 32
  • 40
  • Thanks for the solid suggestions - I have incorporated them with an edit. I am now having trouble with the validation. `validator` is now of type `Reads[Synchable.this.T]` but when I implement it with say `implicit val validator: Reads[Teacher]...` where Teacher extends Synchable, I get an error of incompatible type, "overriding value validator in class Synchable of type play.api.libs.json.Reads[Teacher.this.T]; value validator has incompatible type", which I don't understand. Surely `Teacher.this.T` is compatible with its supertype `Synchable.this.T`? – wwkudu Oct 23 '13 at 08:29
  • It seems the problem I was having when I defined the concrete class for the above was because it is a case of F-bounded Polymorphism (see [here](http://twitter.github.io/scala_school/advanced-types.html), [here](http://stackoverflow.com/questions/16256965/f-bound-polymorphism-with-abstract-types-instead-of-parameter-types), and [here](http://stackoverflow.com/questions/14067406/f-bounded-polymorphism-in-scala) for example). So changed the signature to `abstract class Synchable [T<:Synchable[T]]` and the concrete definition to `class Foo extends Synchable[Foo]` etc. Thanks again. – wwkudu Oct 24 '13 at 07:11