Reading a CSV file using Haskell

Question

I'm reading a CSV using Haskell. I'm not sure if this is the appropriate way to do it.

This is what I'm doing:

1. Read rows from a CSV -> return lazy byte string
2. Parse the headers and rows from the CSV to a tuple -> (headers, [Stock])
3. Remove the headers -> [Stock]
4. Filter the stocks that are "Common Stock" -> [Stock]
5. Print the resulting stocks

Any feedback on how to write better Haskell code is appreciated!

The code is a Stack project, you can find the project and instructions on how to run it: here

I read this section of Stephen Diehl's guide before writing the code: What I wish I knew when learning Haskell

Here is the code to read the CSV file. The main function is printStocks.

{-# LANGUAGE OverloadedStrings #-}

module Lib (printStocks) where

import Control.Monad
import qualified Data.ByteString.Lazy as BL
import Data.Csv
import qualified Data.Vector as V

-- data type to model a stock
data Stock = Stock
  { code :: String,
    name :: String,
    country :: String,
    exchange :: String,
    currency :: String,
    instrumentType :: String
  }
  deriving (Show)

instance FromNamedRecord Stock where
  parseNamedRecord record =
    Stock
      <$> record .: "Code"
      <*> record .: "Name"
      <*> record .: "Country"
      <*> record .: "Exchange"
      <*> record .: "Currency"
      <*> record .: "Type"

-- type synonyms to handle the CSV contents
type ErrorMsg = String

type CsvData = (Header, V.Vector Stock)

-- Function to read the CSV
parseCSV :: FilePath -> IO (Either ErrorMsg CsvData)
parseCSV filePath = do
  contents <- BL.readFile filePath
  return $ decodeByName contents

-- Discard headers from CsvData
removeHeaders :: CsvData -> V.Vector Stock
removeHeaders = snd

-- Check if the given element is a Common Stock
isStock :: Stock -> Bool
isStock stock = instrumentType stock == "Common Stock"

filterStocks :: V.Vector Stock -> V.Vector Stock
filterStocks = V.filter isStock

-- Print the stocks from the CSV file
printStocks :: FilePath -> IO ()
printStocks filePath =
  parseCSV filePath
    >>= print . fmap (filterStocks . removeHeaders)

Answer 1

Looks great overall, especially how you've included instructions on running it, even with sample data, makes this a great submission.

---

The first thing I've noticed, is the error handling for opening files. parseCsv for example checks for the existence of the file - that's a big hint that the function doesn't work in all circumstances as expected, like if the file exists, but isn't readable (try chmod a-r test-resources/empty-file.csv and see how it results in an uncaught exception *** Exception: test-resources/empty-file.csv: openBinaryFile: permission denied (Permission denied)). From the signature I'd actually expect this to be handled via the Either:

parseCsv filePath = do
  result <- try $ BL.readFile filePath
  return $ case result of
             Left (exception :: IOException) -> Left $ show exception
             Right contents -> decodeByName contents

I'm sure that could be done nicer, this one would also need ScopedTypeVariables enabled.

---

IMO Csv looks odd, but the package is already using the name, I guess that's fine.

Some of the comments could be better, like parseCsv saying "Function to read the CSV" - well, yes, we can see that from the name already. Maybe "Read raw CSV data from a file", similar to readStocks.

---

I'd probably inline the local function in filterStocks, because I don't think it adds much clarity over an anonymous function:

filterStocks = V.filter (\instrument -> instrumentType instrument == "Common Stock")

Since you're already using fmap liberally I also thought about the following:

filterStocks = V.filter $ fmap (== "Common Stock") instrumentType

However, I actually think it makes it worse in terms of readability.

Similarly, readStocks with its fmap . fmap is a bit too complicated for me to follow. I'd argue that expanding it a bit might be better for understanding for the next reader.