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 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

removeHeaders :: CsvData -> V.Vector Stock

-- 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)


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.