HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Replacing part of a string with filler characters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fillerwithpartreplacingcharactersstring

Problem

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)])

Solution

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

Code Snippets

main = do
  putStrLn $ foldr (rangedReplace '*') "Fork bombs" [(1, 3), (6, 8)]
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

Context

StackExchange Code Review Q#55953, answer score: 4

Revisions (0)

No revisions yet.