Remove the unnecessary "() =>" in `def iteratorSeq` from com.twitter.cassovary.util.io.GraphReader

[pankajgupta]

It doesn't seem to me that this is necessary. The original idea I guess was to ensure that the files are read during graph construction and not upfront. But even if we just have Seq[Iterator[NodeIdEdgesMaxId]] only the iterators are constructed and not actually traversed. Hence it seems like the extra empty parentheses are un-necessary.

Would you agree @szymonm ?

[szymonm]

Maybe the intention was to open the file on call? Have no idea what else could that be.

It is also puzzling that we don't need to close the file.

Read 2nd answer from: http://stackoverflow.com/questions/4458864/whats-the-right-way-to-use-scala-io-source and it turns out that we should actually avoid scala's Source.fromFile()...

[pankajgupta]

Aha -- not closing the file is definitely a bug that we should fix.

I wouldn't go to the extent to completely avoid io.Source but yes should take care of exceptions

But this will need to be modified too in AdjacencyListGraphReader as this will be evaluated as soon as the class is created: val (edgesBySource, nodeMaxOutEdgeId) = readEdgesBySource()

[szymonm]

That is true, but () => Iterator is still weird. IMO Iterable looks better than this, if some initialization is needed.

I would like to see there sth like

def iteratorSeq: Iterable[NodeStream]

where

NodeStream extends Iterable[NodeIdEdgesMaxId] with Closeable {..}

[pankajgupta]

Agreed Iterable is good.

[AnishShah]

I'm working on this. I did as @szymonm said. Then, do I also have to change Seq[() => Iterator[NodeIdEdgesMaxId]] to Iterable[NodeStream] ?

[AnishShah]

Sorry. I mean, do I have to change Seq[() => Iterator[NodeIdEdgesMaxId]] to Iterable[NodeStream] in ArrayBasedDirectedGraph and SharedArrayBasedDirectedGraph ?

[szymonm]

Pls squash the commits, @AnishShah when this will be ready to merge.

[AnishShah]

@szymonm Done.