meriley
meriley

Reputation: 1881

Issues Overloading CTOR's in Scala

Im having an issue over loading the constructors in Scala. Each time I try to pass a value for the over loaded CTOR I get the error

Example: 
    var client : Client = Client(*variable type List[String]()*);

 Unspecified value parameter clientList.    

My goal is to have an object created using 2 different data types. One a NodeSeq and the Other a List. Never Both. Am I over loading the CTOR correctly or is there a more efficient way to achieve my goal?

package api
import xml._

case class Client(clientNode: NodeSeq, clientList: List[String]) {

  def this(clientNode: NodeSeq) = this(clientNode, null)
  def this(clientList: List[String]) = this(null, clientList)

  var firstName: String
  var lastName: String
  var phone: String
  var street: String
  var city: String
  var state: String
  var zip: String
  var products = List[String]()
  var serviceOrders = List[String]()

  if (clientList == null) {
    firstName = (clientNode \\ "firstname").text
    lastName = (clientNode \\ "lastname").text
    phone = (clientNode \\ "phone").text
    street = (clientNode \\ "street").text
    city = (clientNode \\ "city").text
    state = (clientNode \\ "state").text
    zip = (clientNode \\ "zip").text

    (clientNode \\ "products").foreach(i => products = i.text :: products)

    (clientNode \\ "serviceOrders").foreach(i => serviceOrders = i.text :: serviceOrders)

  } else {
    firstName = clientList(0)
    lastName = clientList(1)
    phone = clientList(2)
    street = clientList(3)
    city = clientList(4)
    state = clientList(5)
    zip = clientList(6)
  }

  override def toString(): String = {
    return "Name : " + firstName + " " + lastName +
      "\nAddress : " +
      "\n\t" + street +
      "\n\t" + city + ", " + state + " " + zip
  }

}

Upvotes: 0

Views: 267

Answers (2)

elbowich
elbowich

Reputation: 1951

Auxiliary constructors are for simple one-liners and aren't suitable in this case. More idiomatic way would be to define factory methods in a companion object:

case class Client(firstName: String,
                  lastName: String,
                  products: List[String] = Nil)

object Client {

  import scala.xml.NodeSeq

  def fromList(list: List[String]): Option[Client] =
    list match {
      case List(firstName, lastName) =>
        Some(Client(firstName, lastName))
      case _ => None
    }

  def fromXml(nodeSeq: NodeSeq): Option[Client] = {
    def option(label: String) =
      (nodeSeq \\ label).headOption.map(_.text)
    def listOption(label: String) =
      (nodeSeq \\ label).headOption.map {
        _.map(_.text).toList
      }
    for {
      firstName <- option("firstname")
      lastName <- option("lastname")
      products <- listOption("products")
    } yield Client(firstName, lastName, products)
  }
}

I also took the liberty of improving your code by eliminating mutability and making it generally more runtime safe.

Upvotes: 0

Rex Kerr
Rex Kerr

Reputation: 167901

You didn't post working code; you can't have undefined vars.

Anyway, the problem is that even though you have overridden the constructors, you have not overridden the builders in the companion object. Add this and it will work the way you want:

object Client {
  def apply(clientNode: NodeSeq) = new Client(clientNode)
  def apply(clientList: List[String]) = new Client(clientList)
}

(if you're using the REPL, be sure to use :paste to enter this along with the case class so you add to the default companion object instead of replacing it).

But a deeper problem is that this is not the way you should be solving the problem. You should define a trait that contains the data you want:

trait ClientData {
  def firstName: String
  def lastName: String
  /* ... */
}

and inherit from it twice, once for each way to parse it:

class ClientFromList(cl: List[String]) extends ClientData {
  val firstName = cl.head
  . . .
}

or you could turn the NodeSeq into a list and parse it there, or various other things. This way you avoid exposing a bunch of vars which are probably not meant to be changed later.

Upvotes: 5

Related Questions