Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add example of incremental, name based parsing #209

Conversation

MaxGabriel
Copy link

There is already an example for indexed-based decoding, but not for name-based.

This also adds a salaries.csv file, which is used by the new code, but incidentally closes #208

There is already an example for indexed-based decoding, but not for name-based.

This also adds a salaries.csv file, which is used by the new code, but incidentally closes haskell-hvr#208
@MaxGabriel MaxGabriel force-pushed the addIncrementalNameBasedDecodeExample branch from 35bcb2e to 25a109c Compare December 20, 2021 21:27
@MaxGabriel
Copy link
Author

Fwiw, I've never used the incremental CSV parsing stuff, so I'm mostly cargo-culting from the indexed based version and puzzling things out/playing type tetris. But, this compiles/runs, so that's a good sign

Copy link
Member

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

main :: IO ()
main = withFile "salaries.csv" ReadMode $ \ csvFile -> do

let headerLoop (FailH _ errMsg) = putStrLn errMsg >> exitFailure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use die errMsg instead? (Also below.)

headerLoop (PartialH fn) = headerLoop =<< feedHeader fn
headerLoop (DoneH header parser) = loop 0 parser

loop !_ (Fail _ errMsg) = putStrLn errMsg >> exitFailure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why !_ and not just _?
One possible explanation: There are errors inside acc that should be brought to surface before crashing. Is that true?

isEof <- hIsEOF csvFile
if isEof
then return $ k B.empty
else k `fmap` B.hGetSome csvFile 4096
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use k <$> ... instead of k ``fmap`` ....

then return $ k B.empty
else k `fmap` B.hGetSome csvFile 4096

feed k = do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feed looks like an identical clone of feedHeader. Please get rid of one of them.

where
sumSalaries :: [Either String SalaryInfo] -> Int
sumSalaries rs =
let good = rights rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to the lefts? Is Left s an error message?
Usually, swallowing errors is not a good idea. These errors should be raised. (If these are errors.)

sumSalaries :: [Either String SalaryInfo] -> Int
sumSalaries rs =
let good = rights rs
in sum $ map (\sinfo -> salary sinfo) good
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary lambda? Does sum $ map salary good also work?

@MaxGabriel MaxGabriel closed this Dec 21, 2021
@andreasabel
Copy link
Member

@MaxGabriel Why did you close the PR now?

@andreasabel
Copy link
Member

ping @MaxGabriel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Example code missing salaries.csv file
2 participants