jesseeichar/scala-io — pull request #74 by mosesn (2012-09-25 03:18:00+00:00)
Title: corrects Path.size per #58

## motivation
Fixes github issue #58, to the extent that it Path.size will return a None if the length is 0. Semantically, it makes more sense to return a None for an empty file, rather than Some(0), since although a file was found, it was trivial.

## issue #58 completeness
Issue #58 mentions that it's unclear whether exists works or not, which is not covered by this pull request. Probably, the issue should be closed, but a new issue should be opened to check that.

## testing
I wasn't sure where to put a test in for this, but it should be tested.

## different behavior
Previously, Path.size returned Some(0) for files that existed but were empty. Now, it will instead return None.

## implementation choices
The existing implementation was a simple one-liner,
```scala
if (jfile.exists()) Some(jfile.length()) else None
```
This was incorrect, but it was unclear how to fix it in a way that preserved its clarity and efficiency. The different ideas I toyed with were
```scala
if (jfile.exists() && jfile.length() != 0) Some(jfile.length()) else None
```
This is the cleanest and the most straightforward, but I think that jfile.length() is not cached, and in the case where the file exists and is nonempty, will have to get a line length twice.

Another idea for how to implement it is to keep this clean system, but instead make it a block.
```scala
{
val length = jfile.length()
if (jfile.exists() && length != 0) Some(length) else None
}
```
However, it requires using a block, which is unsightly.

Ultimately, I decided to go with something that looks ok, and that I guessed the compiler would probably optimize away anyway. The biggest concern was to bind jfile.length() to a variable so that I didn't have to call it twice, so I used a match expression, which lets me both check its value and binds its contents.
```scala
if (jfile.exissts()) jfile.length() match {
case 0 => None
case nonzero => Some(nonzero)
} else None
```
The value of the "if" part of the expression being a match expression is a little messy, which is distasteful, but I felt like adding another set of curly braces was worse.