1
\$\begingroup\$

Challenge: find the top three most frequently used words in a string. Only imports from the base package are allowed (source: codewars)

I'm looking for feedback on (1) readability, and (2) performance

  • Was folding the string into "frequency map" a good choice?
  • I have several small functions that I compose together. Are these abstraction layers helping or hurting readability?
  • How are my functions names?
  • Does the base package have any libraries that would make this code significantly easier to write?
  • Could I benefit from using more pointfree style?
import qualified Data.Map as Map
import Data.List
import Data.Function
import Data.Char
import Data.Maybe

top3 :: [Char] -> [[Char]]
top3 str =
  let
    wordFrequencyMap = foldr (Map.alter increment) Map.empty (normalizedWords str)
    sortedWordFrequencies = sortBy reverseBySecond (Map.toList wordFrequencyMap)
  in map fst (take 3 sortedWordFrequencies)
  where
    normalizedWords = filter containsAtLeastOneAlphaNumericChar . lowerCaseWords . map isLegalChar
    containsAtLeastOneAlphaNumericChar w = find isAlphaNum w /= Nothing
    lowerCaseWords = map (map toLower) . words
    isLegalChar c = if isAlphaNum c || c =='\'' then c else ' '
    reverseBySecond x y = if snd x < snd y then GT else LT
    increment Nothing = Just 1
    increment (Just x) = Just (x + 1)
\$\endgroup\$

3 Answers 3

2
\$\begingroup\$

The frequency map is a good way to solve this. A nice trick to generate the frequency map is by creating a list of singleton maps and merging them all together:

wordFrequencyMap = Map.unionsWith (+) $ map (\x -> singleton x 1) $ normalizedWords str

I think this way is a bit more declarative which is what we aim for in Haskell.


You could simplify:

    containsAtLeastOneAlphaNumericChar w = find isAlphaNum w /= Nothing

to

    containsAtLeastOneAlphaNumericChar = any isAlphaNum

At that point you might find it clearer to inline the definition:

    normalizedWords = filter (any isAlphaNum) . lowerCaseWords . map isLegalChar

To sort in descending order, use Down from Data.Ord, this will reverse the ordering of whatever is passed into it, e.g. Down 1 > Down 2 is True.

    sortedWordFrequencies = sortOn (Down . snd) (Map.toList wordFrequencyMap)
\$\endgroup\$
1
\$\begingroup\$

I'd write

increment = (<|> (Just 1)) . fmap (+1)

You can probably also write

reverseBySecond = flip $ comparing snd

if you don't mind getting EQ for equal pairs.

\$\endgroup\$
1
  • \$\begingroup\$ I like the use of <|>, I've never used it. I'll add it to my toolkit. \$\endgroup\$
    – cgoates
    Sep 11, 2021 at 0:41
-1
\$\begingroup\$

i had already answered this from within my http://lynx.browser.org/ unbloated hypertext browser within superfast http://www.washington.edu/alpine/ mail commander but http://stackexchange.com/ does not behave like written from humans for humans and rather unfortunately insists on a robot captcha unsolvable on a braille typewriter.

i will recreate my first intuitive answer which goes more lambda calculus and less hoare calculus than your code to try

top3=map head.take 3.reverse.sortBy(comparing length).group.sort.words
\$\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.