# Reading and parsing CSV files

This is my first real attempt at a Scala program. I come from a predominantly Java background, so I'd like to know if the program sticks to Scala conventions well.

Is it well readable or should it be formulated differently? To me, there is a lot of lines in the main function which doesn't quite bode right.

It is functioning and I can provide my JUnit tests to prove it. Not all tests pass as those dealing with whitespace are now different.

I didn't want to use any parsing library or support as that doesn't help me learn the core of the language.

package com.wesleyacheson

import scala.io.Source
import scala.annotation.tailrec

val QuoteChar = '"'
val LF = '\n'
val CR = '\r'
val Separator = ','

def readAll(lines:List[List[String]] = Nil):List[List[String]] = {
if (source.hasNext) {
}
return lines.reverse;
}

@tailrec final def readUntilQuote(buffered: AnyRef with BufferedIterator[Char], partial:String = ""):String = {
val char = buffered.next()
val next = if(buffered.hasNext) buffered.head else ""
(char, next) match {
case (QuoteChar, QuoteChar) => readUntilQuote(buffered, partial + buffered.next()) //We've read both charachters so skip
case (QuoteChar, _) => partial
case _ =>  readUntilQuote(buffered, partial + char)
}
}

val buffered = source.buffered
@tailrec def readLine(tokens:List[String] , partialToken:String):List[String] = {
val finished = {() => (partialToken::tokens).reverse}
if (!buffered.hasNext) return finished()
val char=buffered.next()
val subsequentChar = if (buffered.hasNext) buffered.head

(char, subsequentChar) match {
case (QuoteChar, x) if x != QuoteChar =>
case (QuoteChar, QuoteChar) =>
buffered.next();
case (Separator, _) =>
case (CR, LF) =>
buffered.next(); finished()
case (CR, _) =>
finished()
case (LF, _) =>
finished()
case _ =>
}

}

}

}


If interested, the unit tests are here:

package com.wesleyacheson

import org.junit._
import Assert._
import scala.io.Source

val simpleSource = Source.fromString("""abc,def,ghi
jkl, zzz""");
val quotedFields = Source.fromString("foo bar, \"foo bar\"");

val source: Source = null;

try {
fail("Should have thrown exception")
} catch {
case e: IllegalArgumentException => //Expected
}
}

}

}

@Test def VerifyFirstLineContainsExpectedValues() {
println(firstLine)
assertEquals("Checking the number of tokens in the first line", 3, firstLine.size)
assertTrue("Checking that the line contains " + "abc", firstLine.contains("abc"))
assertTrue("Checking that the line contains " + "def", firstLine.contains("def"))
assertTrue("Checking that the line contains " + "ghi", firstLine.contains("ghi"))
assertFalse("Checking that the line does not contain" + "jkl", firstLine.contains("jkl"))
}

@Test def verifyBothQuotedFieldsAreTheSame() {
assertEquals("Checking that the number of tokens in the first line", 2, line.size);
assertEquals("foo bar", line(0));
assertEquals("foo bar", line(1));
}

@Test def verifySimpleQuotedValueIsUnchanged {
}

@Test def verifyDoubleQuotesAreConvertedToQuote {
}

@Test def verify3QuotesAreTreatedAsDoubleQuotesWithinSection {

}

@Test def verifyQuotedCommasAreReturned {
}

}

@Test def verifyTailingWhiteSpaceIsRemoved {
}

}

@Test def verifyQuotedTailingWhitespaceIsPreserved {
}

@Test def verifyQuotedBlankLinesArePreserved{
}

@Test def verifyCellsAreInTheRightOrder{
assertEquals("first", returned(0));
assertEquals("second", returned(1));
}

@Test def verifyRowsAreInTheRightOrder{
assertEquals("first", returned(0)(0));
assertEquals("second", returned(1)(0));
}

@Test def verifyNewLineCarriageReturnIsOnlyTreatedAsOneBlankLine{
assertEquals("first", returned(0)(0));
assertEquals("second", returned(1)(0));
}

}

@Test def verifyThrowsExceptionIfNoQuoteFound {
try {
fail()
} catch {
case e => //expected
}
}
}


What I see when I look at this coming from a Java viewpoint. String concatenation isn't usually done with +. However I don't see how to use a string buffer and keep it semi functional.

The program isn't a functional program. I don't think this could have been avoided using a source. Pure functional programming would have meant that I'd have to make concessions like converting the entire input to a string outside and passing that in which isn't practical.

I don't like the argument-less anonymous function but I don't know what else to do for it.

val finished = {() => (partialToken::tokens).reverse}


I've been told that a lazy val may be more appropriate for this, and I tend to agree.

It may be better for extendability if I returned a list of token objects (or is this my Java head interfering?)

I wonder if any traits could be mixed in to make it more rich. If there was a trait dealing with 2-dimension tabular data for instance.

The buffered iterator was a bit of a cheat. My original was far longer until I added that. Feels like maybe I've skipped a bit of learning for the sake of convenience.

• @Jamal I can't agree with the tag edits. This may be a case of reinventing the wheel but the purpose of it is learning. The focus of the question isn't about how to prevent that. Similarly for csv tag, and the unit-testing. The question isn't about that. The purpose isn't to read a CSV. The tests were only in the question to prove the criteria of code review. The body changes are fine, but I'd like to revert the title and tags. – Athas Aug 15 '14 at 10:44
• Then you may make these changes. – Jamal Aug 15 '14 at 13:40

Here are a few things, going from smallest to the more significant.

1. Try to avoid using return. It isn't idiomatic, and for good reason. So instead of writing if (cond) return a; return b you should prefer if (cond) a else b
2. Unless I'm missing anything, in readUntilQuote's signature buffered should be of type BufferedIterator[Char] instead of AnyRef with BufferedIterator[Char]. The AnyRef there doesn't do anything useful.
3. You can use Source and still have functional code. Specifically, try looking at Source.getLines(). It returns an Iterator[String] that is lazily-evaluated but can still be used functionally almost like other collections (you can map, fold, filter, etc.)

Below you can see that I've rewritten a bit of your code. For readabilities sake my first suggestion is to attempt to make your function definitions and their parameter lists consistent with the Scala Style Guide. Next, I would also move the @tailrec annotation one line above the function for which it is intended. Lastly, I would rename your inner function such that people reading your code can quickly see that it is an inner 'Helper' function.

    def readLine(): List[String] = {
// ...

@tailrec
def readLineHelper(tokens: List[String], partialToken: String): List[String] = {
// function body
}


You also mentioned that you didn't like your finished function. It appears to me that you can improve its implementation in one of two ways. At first glance it seems that readLineHelpers parameters partialToken and tokens are never altered throughout the course of the function, in other words they are immutable values. If this is the case than finished can be declared as a reference to data:
    val finished = (partialToken :: tokens) reverse

If, however, there is some state change to partialToken or tokens then you will need to declare finished as you originally did, as a function value but with the following syntax:
    val finished = () => (partialToken :: tokens) reverse