CODEWITHSUNDEEP
scalajdbcakkaactor
sc_ray
sc_ray

Reputation: 8043

scala+jdbc+case class+actors design confusion

I am writing an exporter that will take results from the database and take every individual records and write it to a comma separated file. Different queries will have different worker created for it since they need to write separate csv files. To start off, I have decoupled the tasks into two different actors. Actor1 is a JdbcWorker which queries the database provided a query parameter and Actor2 is a CSVWriter which receives case class representing the result from the query that needs to be appended to the CSV. My first question is, even though I like the separation of concerns provided by these two workers but is it good design to decouple the jdbc query from the CSV writer?

So, I have written actor1 as follows:

class DataQueryWorker(csvExporterWorker: ActorRef) extends Actor with ActorLogging{

  private implicit def ModelConverter(rs: ResultSet): QueryModel = {
    QueryModel(
      id = rs.getString(0),
      name = rs.getString(1),
      age = rs.getString(2),
      gender = rs.getString(3))
}

  private def sendModelToCsvWorker(model: QueryModel): Unit = {
    csvExporterWorker ! model
  }

  private def startExport[T](queryString: String)(resultFunc: T => Unit)(implicit ModelConverter: ResultSet => T): Unit = {
    try {
      val connection = DriverManager.getConnection(DbConfig.connectionString,
        DbConfig.user,
        DbConfig.password)
      val statement = connection.createStatement(java.sql.ResultSet.TYPE_FORWARD_ONLY, java.sql.ResultSet.CONCUR_READ_ONLY)
      statement.setFetchSize(Integer.MIN_VALUE)
      val rs = statement.executeQuery(queryString)
      while (rs.next()) {
        resultFunc(ModelConverter(rs))
      }
    } catch {
      case e: Exception => //What to do in case of an exception???
    }
  }

  override def receive() = {
    case startEvent => startExport(DbConfig.ModelExtractionQuery)(sendModelToCsvWorker)
  }

}

My next question would be, is the code written above, the proper way to query the database, wrap it in a model and send the result to the CSVWorker? I am not sure if I am following the scala idioms properly. Also, what would be the proper way to handle exceptions in this case?

It will be great to get some guidance on this.

Thanks

Upvotes: 0

Views: 808

Answers (2)

James_pic
James_pic

Reputation: 3307

I think using actors here is probably overkill.

Actors are useful when you want to operate on mutable state with multiple threads safely. But, in your case, you say that each query writes to a separate CSV file (so there's only one thread per CSV file). I don't think the CSVWorker actor is necessary. It could even potentially be harmful, as the actor mailbox could grow and consume a significant amount of memory, if the DBWorker is signifcantly faster than the CSVWorker.

Personally, I'd just call the CSV writer directly.

The question about separation of concerns depends on whether you expect this code to be re-used in unrelated contexts. If you're likely to want to use your JDBC worker with other writers, then it may be worth it (although there's a school of thought that says you're better off waiting until a need arises before refactoring - You Aint Gonna Need It or YAGNI). Otherwise, you might be better off simplifying.

If you do decide to attach the JDBC code to the CSV code directly, you might also want to take out the case class conversion. Again, if this is code that will be re-used elsewhere, then it's better to keep it.

Exception handling depends on your application, but in Scala (unlike in Java), if you don't know what to do about an Exception, you probably shouldn't do anything. Take the try..catch block out, and just let the exception propagate - something will catch it, and report it.

Java forces you to handle exceptions, which is a great idea in theory, but in practice often leads to error handling code that does nothing of any real use (either re-throwing, or worse, swallowing errors).

Oh, and if you're writing a lot of code that turns ResultSets into case classes, and vice versa, you might want to look at using an Object Relation Mapping framework, like Slick or Squeryl. They're optimised for precisely this use case.

Upvotes: -1

cmbaxter
cmbaxter

Reputation: 35453

I think your approach is ok with a couple of minor changes:

For the DB actor, you might want to look into making these long lived actors, pooled behind a Router. Let this actor hold a Connection as it's state, opening it once when started and closing then reopening in case of restart due to failure. I think this might be a better approach as you won't always need to be opening connections for calls to export data. You just need to write some code for perhaps checking the state of the connection (and reconnecting) before making calls to it.

Once you make the DB actor stateful and long lived, you won't be able to pass the CSVWorker in via the constructor. You should instead pass it in via the message to this actor indicating that you want an export. You could do that via a case class like so:

case class ExportQuery(query:String, csvWorker:ActorRef)

Change your receive to look like this:

def receive = {
  case ExportQuery(query, csvWorker) =>
    ...
}

And lastly, remove the try/catch logic. Unless you can do something meaningful based on this failure (like call some alternate code path) it doesn't make sense catching it. Let the actor fail and get restarted (and close/reopen the connection) and move on.

Upvotes: 2

Related Questions