From 30b1c10038c463af3b1fcbc13fca39a688c9f4d0 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Sat, 21 Dec 2013 21:16:28 -0500 Subject: [PATCH] Return an Either from parse_xmlfid instead of a Maybe for better error reporting. --- htsn.cabal | 2 ++ src/Main.hs | 32 ++++++++++++++++++++++++-------- src/TSN/Xml.hs | 43 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/htsn.cabal b/htsn.cabal index 55dca86..8aa384f 100644 --- a/htsn.cabal +++ b/htsn.cabal @@ -24,6 +24,7 @@ executable htsn filepath == 1.3.*, hslogger == 1.2.*, hxt == 9.3.*, + MissingH == 1.2.*, network == 2.4.*, tasty == 0.5.*, tasty-hunit == 0.4.*, @@ -73,6 +74,7 @@ test-suite testsuite filepath == 1.3.*, hslogger == 1.2.*, hxt == 9.3.*, + MissingH == 1.2.*, network == 2.4.*, tasty == 0.5.*, tasty-hunit == 0.4.*, diff --git a/src/Main.hs b/src/Main.hs index f36e2ba..4ebada0 100644 --- a/src/Main.hs +++ b/src/Main.hs @@ -55,29 +55,45 @@ import TSN.FeedHosts ( FeedHosts(..) ) import TSN.Xml ( parse_xmlfid ) --- | Warning! This does not automatically append a newline. The output --- is displayed/logged as-is, for, you know, debug purposes. +-- | Display and log debug information. WARNING! This does not +-- automatically append a newline. The output is displayed/logged +-- as-is, for, you know, debug purposes. report_debug :: String -> IO () report_debug s = do display_debug s log_debug s + +-- | Display and log an error condition. This will prefix the error +-- with "ERROR: " when displaying (but not logging) it so that it +-- stands out. +-- report_error :: String -> IO () report_error s = do display_error $ "ERROR: " ++ s log_error s + +-- | Display and log an informational (status) message. report_info :: String -> IO () report_info s = do display_info s log_info s --- | Warning! This does not automatically append a newline. + +-- | A special case of report_debug for reporting the two bits of data +-- that we sent to TSN: the username and password. +-- report_sent :: String -> IO () report_sent s = do display_sent s log_debug s + +-- | Display and log a warning. This will prefix the warning with +-- "WARNING: " when displaying (but not logging) it so that it +-- stands out. +-- report_warning :: String -> IO () report_warning s = do display_warning $ "WARNING: " ++ s @@ -103,10 +119,9 @@ recv_line h = do -- save_document :: Configuration -> String -> IO () save_document cfg doc = - case maybe_path of - Nothing -> - report_error "Document missing XML_File_ID element." - Just path -> do + case either_path of + Left err -> report_error err + Right path -> do already_exists <- doesFileExist path when already_exists $ do let msg = "File " ++ path ++ " already exists, overwriting." @@ -114,9 +129,10 @@ save_document cfg doc = writeFile path doc report_info $ "Wrote file: " ++ path ++ "." where + -- All the fmaps are because we're working inside a Maybe. xmlfid = fmap show (parse_xmlfid doc) filename = fmap (++ ".xml") xmlfid - maybe_path = fmap ((output_directory cfg) ) filename + either_path = fmap ((output_directory cfg) ) filename -- | Loop forever, writing the buffer to file whenever a diff --git a/src/TSN/Xml.hs b/src/TSN/Xml.hs index 4c1123f..9d83f58 100644 --- a/src/TSN/Xml.hs +++ b/src/TSN/Xml.hs @@ -6,7 +6,7 @@ module TSN.Xml ( xml_tests ) where -import Data.Maybe ( listToMaybe, mapMaybe ) +import Data.Either.Utils ( maybeToEither ) import Test.Tasty ( TestTree, testGroup ) import Test.Tasty.HUnit ( (@?=), Assertion, testCase ) import Text.Read ( readMaybe ) @@ -19,11 +19,32 @@ import Text.XML.HXT.Core ( runLA, xreadDoc ) + -- | A tiny parser written in HXT to extract the "XML_File_ID" element --- from a document. -parse_xmlfid :: String -> Maybe Integer -parse_xmlfid = - listToMaybe . mapMaybe readMaybe . parse +-- from a document. If we fail to parse an XML_File_ID, we return +-- the reason wrapped in a 'Left' constructor. The reason should be +-- one of two things: +-- +-- 1. No XML_File_ID elements were found. +-- +-- 2. An XML_File_ID element was found, but it could not be read +-- into an Integer. +-- +-- We use an Either rather than a Maybe because we do expect some +-- non-integer XML_File_IDs. In the examples, you will see +-- NHL_DepthChart_XML.XML with an XML_File_ID of "49618.61" and +-- CFL_Boxscore_XML1.xml with an XML_File_ID of "R28916". According +-- to Brijesh Patel of TSN, these are special category files and not +-- part of the usual feed. +-- +-- We want to report them differently, "just in case." +-- +parse_xmlfid :: String -- ^ The XML Document + -> Either String Integer +parse_xmlfid doc = + case parse_results of + [] -> Left "No XML_File_ID elements found." + (x:_) -> x where parse :: String -> [String] parse = @@ -33,6 +54,14 @@ parse_xmlfid = >>> getChildren >>> getText) + read_either_integer :: String -> Either String Integer + read_either_integer s = + let msg = "Could not parse XML_File_ID" ++ s ++ " as an integer." + in + maybeToEither msg (readMaybe s) + + elements = parse doc + parse_results = map read_either_integer elements -- * Tasty Tests @@ -53,5 +82,7 @@ xml_file_id_tests = check xmlfid = do xml <- readFile ("test/xml/" ++ xmlfid ++ ".xml") let actual = parse_xmlfid xml - let expected = readMaybe xmlfid + -- The maybeToEither should always succeed here, so the error + -- message goes unused. + let expected = maybeToEither "derp" (readMaybe xmlfid) actual @?= expected -- 2.43.2