6
\$\begingroup\$

I came up with this function rangedReplace as an exercice to use recursion and try different ways to make it work (I made at least 3 completely different versions). I think this version is the best but I have a feeling I can improve the way the functions/arguments are handled especially in the censor and in the main functions. How can I get these functions to be more idiomatic Haskell-way of doing, ?

-- Replace the letters in the range [from:to[ ("to" not included) with chr
rangedReplace :: Int -> Int -> Char -> String -> String
rangedReplace from to chr str 
    | from == to = str
    | otherwise  = (take from' str) ++ rangedReplace' chr n (drop from' str)
    where from' = min from to
          to'   = max from to
          n     = to' - from'

-- Helper function
rangedReplace' :: Char -> Int -> String -> String
rangedReplace' chr n str
    | n == 0    = str
    | str == "" = ""
    | otherwise = chr : (rangedReplace' chr (n-1) (tail str))

Usage example:

censor (from,to) = rangedReplace from to '*'

main = do
    putStrLn $ foldr id "Fork bombs" (map censor [(1,3),(6,8)]) 
\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

Parameter design

To facilitate currying, you should arrange a function's parameters starting with the one that is the least likely to vary. In this case, I would consider the fill character the parameter that is most likely to be fixed.

The next parameter, I think, should be the range, and it should be specified as a pair rather than as two separate parameters. That would make rangedReplace fill (from, to) a filter that takes a string and produces a string.

Using inclusive bounds for from and exclusive bounds for to is the right way to go.

I don't think it's beneficial to automatically swap from and to. That just encourages your caller to be sloppy. Rather, I would expect a rangedReplace from 3 to 1 to act as a no-op.

With rangedReplace fill (from, to) string, your main function no longer needs id, map, and the censor helper function.

main = do
  putStrLn $ foldr (rangedReplace '*') "Fork bombs" [(1, 3), (6, 8)]

Implementation

The use of ++ should be considered a yellow flag, as it indicates a full list traversal. You'll be doing three O(from') operations: take, drop, and ++.

I'd rather go with a recursive solution that does everything in one pass, and is fully lazy. The implementation is shorter, too.

rangedReplace :: Char -> (Int, Int) -> String -> String
rangedReplace _ _ [] = []
rangedReplace fill (from, to) s@(c:cs)
  | to <= 0   = s
  | from <= 0 = fill:cs'
  | otherwise = c:cs'
  where cs' = rangedReplace fill ((from - 1), (to - 1)) cs
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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