Skip to main content
Address and refactor second function as well
Source Link
tarleb
  • 308
  • 3
  • 9

A very good solution has already been given by above. I'd like to add some comments on the current code without introducing a new algorithm.

getNextChar :: Sequence -> Char -> Char getNextChar s c | c == getLast s = getFirst s | otherwise = findChar (getSequence s) where findChar (x:r@(y:xs)) | x == c = y | otherwise = findChar r 

findChar's name doesn't quite capture its intend. It would more aptly be better named successorChar or something similar. The variable xs isn't used in the function, one could get away without binding the remaining list to a variable ((x:r@(y:_))). A much simpler way to write it would be to use dropWhile:

findChar xs = head . tail . dropWhile (/= c) $ xs 

One could get rid of the special case testing if we are looking at the last char by making the sequence of chars cyclic:

nextChar s c = head . tail . dropWhile (/= c) $ cycle (getSequence s) 

The main function suffers from the way our problem is posed. Everything would be much simpler if the sequence was reversed (meaning that 'aa' would be followed by 'ba'). This allows us to replace list appends (++) with more efficient (:) and to do pattern matching on the chars that matter.

seqSuccessor s [] = getFirst s seqSuccessor s cs'@(c:cs) = | c /= getLast s = (getNextChar s c):cs | otherwise = let (ls, ts) = span (== (getLast s)) cs' in (replicate (length ls) $ getFirst s) ++ seqSuccessor s ts 

The original function can be recovered simply by doing some list reversions:

getNextStr s = reverse . seqSuccessor s . reverse 

A very good solution has already been given by above. I'd like to add some comments on the current code without introducing a new algorithm.

getNextChar :: Sequence -> Char -> Char getNextChar s c | c == getLast s = getFirst s | otherwise = findChar (getSequence s) where findChar (x:r@(y:xs)) | x == c = y | otherwise = findChar r 

findChar's name doesn't quite capture its intend. It would be better named successorChar or something similar. The variable xs isn't used in the function, one could get away without binding the remaining list to a variable ((x:r@(y:_))). A much simpler way to write it would be to use dropWhile:

findChar xs = head . tail . dropWhile (/= c) $ xs 

One could get rid of the special case testing if we are looking at the last char by making the sequence of chars cyclic:

nextChar s c = head . tail . dropWhile (/= c) $ cycle (getSequence s) 

A very good solution has already been given above. I'd like to add some comments on the current code without introducing a new algorithm.

getNextChar :: Sequence -> Char -> Char getNextChar s c | c == getLast s = getFirst s | otherwise = findChar (getSequence s) where findChar (x:r@(y:xs)) | x == c = y | otherwise = findChar r 

findChar's name doesn't quite capture its intend. It would more aptly be named successorChar or something similar. The variable xs isn't used in the function, one could get away without binding the remaining list to a variable ((x:r@(y:_))). A much simpler way to write it would be to use dropWhile:

findChar xs = head . tail . dropWhile (/= c) $ xs 

One could get rid of the special case testing if we are looking at the last char by making the sequence of chars cyclic:

nextChar s c = head . tail . dropWhile (/= c) $ cycle (getSequence s) 

The main function suffers from the way our problem is posed. Everything would be much simpler if the sequence was reversed (meaning that 'aa' would be followed by 'ba'). This allows us to replace list appends (++) with more efficient (:) and to do pattern matching on the chars that matter.

seqSuccessor s [] = getFirst s seqSuccessor s cs'@(c:cs) = | c /= getLast s = (getNextChar s c):cs | otherwise = let (ls, ts) = span (== (getLast s)) cs' in (replicate (length ls) $ getFirst s) ++ seqSuccessor s ts 

The original function can be recovered simply by doing some list reversions:

getNextStr s = reverse . seqSuccessor s . reverse 
Source Link
tarleb
  • 308
  • 3
  • 9

A very good solution has already been given by above. I'd like to add some comments on the current code without introducing a new algorithm.

getNextChar :: Sequence -> Char -> Char getNextChar s c | c == getLast s = getFirst s | otherwise = findChar (getSequence s) where findChar (x:r@(y:xs)) | x == c = y | otherwise = findChar r 

findChar's name doesn't quite capture its intend. It would be better named successorChar or something similar. The variable xs isn't used in the function, one could get away without binding the remaining list to a variable ((x:r@(y:_))). A much simpler way to write it would be to use dropWhile:

findChar xs = head . tail . dropWhile (/= c) $ xs 

One could get rid of the special case testing if we are looking at the last char by making the sequence of chars cyclic:

nextChar s c = head . tail . dropWhile (/= c) $ cycle (getSequence s)