Advice to improve some code?

5 replies

Mon, 2009-02-02, 03:43

Daniel Wellman

Joined: 2009-02-02,

In trying to get better at Scala, I've been trying to work on some small
exercises. Would someone more experienced mind looking at what I've written
and advising if there is a better "Scala way"?

The problem is this: Given some plain text files describing people in a few
different formats, write a program to import and print out all the people.

Here are a few sample input formats:

Input format: Comma separated
# Last name, First Name, Gender, Favorite Color, Birthdate
Anson, Nigel, Male, Tan, 2/16/1941
Bellevue, Tara, Female, Yellow, 4/24/1976

Input format; Pipe separated
# First name, Last Name, Middle Initial, Gender, Birthdate
Stan | Smythe | D | M | Blue | 2-3-1992
Barbara | Sanderson | A | F | Purple | 7-21-1962

I am using the following code to convert the list of files to a List of
Person objects.

case class Person

val inputFormats = List( new CommaParser(Source.fromFile("...")), new
PipeParser(Source.fromFile("...")) )
val allPeople = inputFormats.flatMap { _.parse }
// Sorting and printing the people omitted

Here's the start of the parsing code. I am not using the parser combinator
library since that seemed to be tuned to parsing languages - not necessarily
data. (Is this correct?)

// A base class, using the template method pattern

abstract class FormatParser {
val token: String // The token to split
val input: Source // The lines to parse

// The parse method is abstract, defined per each subclass
def createPerson(tokens: List[String]): Person

def parse(): List[Person] = {
input.getLines.map(parseLine).toList
}

private def parseLine(input: String) = {
val components = input.split(token).map{ _.trim }
createPerson(components.toList)
}
}

I made the Parsers as classes using the template method pattern; the base
class had the functionality to take a list of Strings read from a file,
tokenize based upon the abstract separator, and map each line to a Person
using the abstract parseLine function. Subclasses must implement the token
abstract value as well as the createPerson function.

This use of the template method pattern felt very Java-ish; is there a more
Scala-ish ways to represent this functionality?

// Parser implementations provide the separator token,
// a date format, and a method to convert a list of String tokens
// into a Person object

class CommaParser(val input: Source) extends FormatParser {
val token = ","
val dateFormat = new SimpleDateFormat("m/d/yyyy")

def createPerson(tokens: List[String]) = tokens match {
case lastName :: firstName :: gender :: favoriteColor :: date :: Nil =>
new Person(lastName, firstName, "", gender, favoriteColor,
dateFormat.parse(date))
case _ => throw new IllegalArgumentException("Does not parse!")
}
}

class PipeParser(val input: Source) extends FormatParser {
val token = "\\|"
val dateFormat = new SimpleDateFormat("m-d-yyyy")

def createPerson(tokens: List[String]) = tokens match {
case lastName :: firstName :: middle :: gender :: favoriteColor :: date
:: Nil =>
new Person(lastName, firstName, middle, gender, favoriteColor,
dateFormat.parse(date))
case _ => throw new IllegalArgumentException("Does not parse!")
}
}

Here I use pattern matching to name the fields in the order they appear in
the input text file format.

Can anyone provide me any advice or suggestions as to how to improve this
code? Thank you for your feedback!