4
\$\begingroup\$

I need to create an instance of Show of a somewhat complicated type that represents a C++ function. The "serialization" gives me a Haskell interpretation of the said function.

It works as I wish but it looks utterly horrible, with the string concatenations, point-free conversions and nested let bindings. How could I improve readability of the code?

instance Show Function where
    show (Function templateArgs returnT fname fArgs) =
        let toLowerIfPolymorphic argName =
               let lcArg = strToLower argName in
                    if lcArg `elem` (map snd templateArgs)
                    then lcArg
                    else id argName in
        concat [
            fname, " :: (",
            intercalate ", " (
                filter (/="") . map (
                \(concept, tIdent) ->
                    if not $ elem concept ["Any", "typename", "class"]
                    then concept ++ " " ++ strToLower tIdent
                    else ""
                ) $ templateArgs    
            ), ") => ",
            intercalate " -> " (
                map toLowerIfPolymorphic . map (show . fst) $ fArgs
            ),
            (if not $ null fArgs then " -> " else ""),
            toLowerIfPolymorphic . show $ returnT
        ]

The Function is:

data Function = Function {
                    funcTemplateArgs :: [(Concept, Identifier)],
                    funcReturnT      :: CppType,
                    funcName         :: String,
                    funcArgs         :: [(CppType, Identifier)]
                }

CppType is basically a sum-type representing Scalar and Parameterzied types. Just synonyms for tuples. Concept / Identifier is a String synonym. Everything is an instance of Show.

Also I find the if condition then f else id very strange. "Do nothing if false" is somewhat silly.

\$\endgroup\$
1
\$\begingroup\$

I would clean this up by bringing what you can to the top level (hiding it using a module export list if need be) as new functions. There are also some sectioning/alias changes that I think would make it more readable.

toLowerIfPolymorphic :: [(Concept, Identifier)] -> Identifier -> Identifier
toLowerIfPolymorphic templateArgs argName
    | lcArg `elem` (snd $ unzip templateArgs) = lcArg
    | otherwise                               = argName
  where
    lcArg = strToLower argName

I often prefer guards to if _ then _ else _, it helps cut down on word soup (and I still don't understand why we don't have boolCase :: Bool -> a -> a -> a in the Prelude). Consider how snd . unzip more specifically captures your intent than using a more powerful but generic higher-order function like map.

The bit with ["Any", "typename", "class"] should work out better with some reordering. Instead of applying a partial transformation with a sentinel value (i.e. filter (not . null) with occasional "" values), drop the junk and then transform the good stuff.

showConstraints :: [(Concept, Identifier)] -> [String]
showConstraints templateArgs = map display . filter valid $ templateArgs
  where
    display (concept, ident) = concept ++ " " ++ strToLower ident
    valid (concept, _) = concept `notElem` ["Any", "typename", "class"]

Then your Show instance ends up looking something like this.

instance Show Function where
  show (Function templateArgs returnT fName fArgs)
     = fName
    ++ " :: ("
    ++ intercalate ", " (showConstraints templateArgs)
    ++ ") => "
    ++ intercalate " -> " (map (toLowerIfPolymorphic templateArgs . show . fst) $ fArgs)
    ++ (if null fArgs then "" else " -> ")
    ++ toLowerIfPolymorphic templateArgs (show returnT)

I'd probably take this one step further though to handle not having any constraints (in which case you don't want "() =>" to show up, presumably) and write one more helper function.

wrap :: String -> String -> String -> String
wrap _     ""      _   = ""
wrap start content end = start ++ content ++ end

Giving—

instance Show Function where
  show (Function templateArgs returnT fName fArgs)
      =  fName
      ++ " :: "
      ++ wrap "(" (intercalate ", "   $ showConstraints templateArgs)       ") => "
      ++ wrap ""  (intercalate " -> " $ map (lowerPoly . show . fst) fArgs) " -> "
      ++ lowerPoly (show returnT)
    where
      lowerPoly = toLowerIfPolymorphic templateArgs
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.