0
\$\begingroup\$

This trait is meant to be a simple interface for tabular data from different file types, such as csv, excel, open office etc. Right now I'm only asking about some of the top level code for now and not the implementations. I'm worried that I might have made some poor design choices. But first let's get some type aliases out of the way from the package object.

package.scala

package object sheets {

  /** Representation of a cell or element.
    * All are strings.
    */
  type Cell = String

  /** Row is usually a Vector of [[Cell]]s */
  type Row = IndexedSeq[Cell]

  /** Column is identical to a [[Row]], the difference is purely contextual. */
  type Column = Row

  /** Table is an indexed sequence of Rows. */
  type Table = IndexedSeq[Row]

  /** Indexes are integers. */
  type Index = Int

  /** Column headers are strings. */                                                                              
  type Field = String
}

Initially, I had Cell as Option[Any] with an empty cell as None but I thought it might be simpler to just represent everything as a string and let the client code parse it as is later. I'm also concerned that declaring Row and Table to be IndexedSeqs instead of plain Iterables might have a memory cost if the client code loads a large file.

Next is the actual trait to be implemented and the companion object that decides which actual implementation to use.

DataSheet.scala

/** Simplified representation of tabular data.
  */
trait DataSheet extends Table {

  /** The collection of values as a 2D vector. */
  val rows: Table

  /** Returns the row at the given index.
    *
    * Literally the same as indexing [[rows]]
    *
    * @param  rowIndex  Index of the row.
    * @return the row at the provided index.
    */
  def rowAt(rowIndex: Index): Row = {
    rows(rowIndex)
  }

  /** Returns the column at the given index.
    *
    * Returns a [[Column]] built from elements of the same index from [[rows]].
    *
    * @param  colIndex  Index of the column.
    * @return The column at the given index.
    */
  def colAt(colIndex: Index): Row = {
    rows.map(cellAt(colIndex))
  }

  /** Returns a subtable made from the rows of the given indexes.
    *
    * Creates a new [[Table]] from the rows of the given indexes, preserving order.
    * For example, getting the rows at indexes 0, 1, and 4 will return a three
    * row table made from the first, second and fifth rows.
    *
    * @param  rows  Indexes of the desired rows.
    * @return A table made from the selected rows.
    */
  def rowsAt(rowIndexes: Iterable[Index]): DataSheet = {
    Sheet(rowIndexes.toVector.map(rows.apply _))
  }

  /** Returns a subtable made from the columns of the given indexes.
    *
    * Identical to [[rowsAt]] but uses the columns of the given indexes not
    * the rows.  It should not transform the columns into rows.
    *
    * @param  cols  Indexes of the desired columns.
    * @return A table made from the selected columns.
    */
  def colsAt(colIndexes: Iterable[Index]): DataSheet = {
    val colVec = colIndexes.toVector
    val cols = rows map { row => 
      colVec map { colIndex =>
        cellAt(colIndex)(row)
      }
    }
    Sheet(cols)
  }

  private def cellAt(colIndex: Index)(row: Row): Cell = {
    if (row.isDefinedAt(colIndex)) row(colIndex)
    else ""
  }

  /**********\
   IndexedSeq
  \**********/

  /** Returns the row at the given index
    *
    * @param  rowIndex  Index of the desired row.
    * @return The row at rowIndex.
    */
  def apply(rowIndex: Index): Row = {
    rowAt(rowIndex)
  }

  // for IndexedSeq
  def length = {
    rows.length
  }
}

/** Simplest implementation of [[DataSheet]]. */
case class Sheet(rows: Table) extends DataSheet

/** Factory object for [[DataSheet]] */
object DataSheet {

  /** Returns a [[DataSheet]] from agiven file URL.
    * 
    * @param url  URL to a data file.
    * @return A new [[DataSheet]] instance.
    */
  def apply(url: URL): DataSheet = {
    val ext = url.toString.split('.').last
    val istream = url.openStream
    try {
      extFactory(ext)(istream)
    } catch {
      case (nsee: NoSuchElementException) => {
        val msg = s".$ext files are not a supported extension"
        throw new UnsupportedOperationException(msg)
      }
      case (e: Exception) => throw e
    } finally {
      istream.close
    }
  }

  /** Returns an [[DataSheet]] from the given file path.
    *
    * @param  path  Path to a data file.
    * @return A new [[DataSheet]] instance.
    */
  def apply(path: Path): DataSheet = {
    apply(path.toUri.toURL)
  }

  def apply(table: Table): DataSheet = {
    Sheet(table)
  }

  private type Factory = InputStream => DataSheet
  private val extFactory = Map[String, Factory](
    "xlsx" -> ExcelSheet.fromXlsxInput(0), // curried like `txt`
    "xls"  -> ExcelSheet.fromXlsInput(0),  // also assumes first sheet in workbook
    "csv"  -> txt(','),
    "ttx"  -> txt('\t'),
    "txt"  -> txt('\t'), 
    "ods"  -> ODSSheet.fromInput(0)
  )

  private def txt(delim: Char)(istream: InputStream): DataSheet = {
    import scala.io.Source
    CSVSheet.fromSource(Source.fromInputStream(istream), colSep=delim)
  }
}

I can post some of the implementations if you want. I thought I would keep it focused for now though.

\$\endgroup\$
2
\$\begingroup\$

This isn't meant to be a complete answer, but here a a few points to start with:

  • When throwing an exception inside a catch clause, you should probably supply a cause for the exception

    throw new UnsupportedOperationException(msg, nsee)

  • Iterable has a map method, you don't need to convert it to a Vector.

    Edit: If you need an IndexedSeq, simply declare your methods to take an IndexedSeq and let the caller do the conversion

  • Multiple parameter lists are usually used to guide the type inference or if you need implicit parameters. I would change these methods: private def cellAt(colIndex: Index)(row: Row): Cell private def txt(delim: Char)(istream: InputStream): DataSheet

\$\endgroup\$
  • \$\begingroup\$ Noted with the error reason. I use toVector because Table is of type IndexedSeq so I need to convert it to one, but I'm hesitant if that is wise. For the parameter lists, where are you suggesting that I apply that? \$\endgroup\$ – cheezsteak Jul 25 '16 at 21:43
  • 1
    \$\begingroup\$ please see my edit \$\endgroup\$ – corvus_192 Jul 26 '16 at 8:31
  • \$\begingroup\$ So you would not multiple parameter lists for those methods? \$\endgroup\$ – cheezsteak Jul 26 '16 at 18:18
  • \$\begingroup\$ No, I wouldn't, because I think there's no reason to use them. \$\endgroup\$ – corvus_192 Jul 28 '16 at 6:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.