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.