0

So I'm trying to make a little program that can take in data captured during an experiment, and for the most part I think I've figured out how to recursively take in data until the user signals there is no more, however upon termination of data taking haskell throws Exception: <<loop>> and I can't really figure out why. Here's the code:

readData :: (Num a, Read a) => [Point a] -> IO [Point a]
readData l = do putStr "Enter Point (x,y,<e>) or (d)one: "
                entered <- getLine
                if (entered == "d" || entered == "done")
                    then return l
                    else do let l = addPoint l entered
                            nl <- readData l
                            return nl

addPoint :: (Num a, Read a) => [Point a] -> String -> [Point a]
addPoint l s = l ++ [Point (dataList !! 0) (dataList !! 1) (dataList !! 2)]
    where dataList = (map read $ checkInputData . splitOn "," $ s) :: (Read a) => [a]

checkInputData :: [String] -> [String]
checkInputData xs
    | length xs < 2 = ["0","0","0"]
    | length xs < 3 = (xs ++ ["0"])
    | length xs == 3 = xs
    | length xs > 3 = ["0","0","0"]

As far as I can tell, the exception is indication that there is an infinite loop somewhere, but I can't figure out why this is occurring. As far as I can tell when "done" is entered the current level should simply return l, the list it's given, which should then cascade up the previous iterations of the function.

Thanks for any help. (And yes, checkInputData will have proper error handling once I figure out how to do that.)

Rigel
  • 31
  • 5

1 Answers1

4

<<loop>> basically means GHC has detected an infinite loop caused by a value which depends immediately on itself (cf. this question, or this one for further technical details if you are curious). In this case, that is triggered by:

else do let l = addPoint l entered

This definition, which shadows the l you passed as an argument, defines l in terms of itself. You meant to write something like...

else do let l' = addPoint l entered

... which defines a new value, l', in terms of the original l.

As Carl points out, turning on -Wall (e.g. by passing it to GHC at the command line, or with :set -Wall in GHCi) would make GHC warn you about the shadowing:

<interactive>:171:33: warning: [-Wname-shadowing]
    This binding for ā€˜l’ shadows the existing binding
      bound at <interactive>:167:10

Also, as hightlighted by dfeuer, the whole do-block in the else branch can be replaced by:

readData (addPoint l entered)

As an unrelated suggestion, in this case it is a good idea to replace your uses of length and (!!) with pattern matching. For instance, checkInputData can be written as:

checkInputData :: [String] -> [String]
checkInputData xs = case xs of
    [_,_] -> xs ++ ["0"]
    [_,_,_] -> xs
    _ -> ["0","0","0"]

addPoint, in its turn, might become:

addPoint :: (Num a, Read a) => [Point a] -> String -> [Point a]
addPoint l s = l ++ [Point x y z]
    where [x,y,z] = (map read $ checkInputData . splitOn "," $ s) :: (Read a) => [a]

That becomes even neater if you change checkInputData so that it returns a (String, String, String) triple, which would better express the invariant that you are reading exactly three values.

Community
  • 1
  • 1
duplode
  • 33,731
  • 7
  • 79
  • 150
  • 1
    Probably also worth noting that adding the `-Wall` flag would have pointed out the shadowing, giving a strong clue what was wrong. – Carl Jan 08 '17 at 04:38
  • 1
    Indeed, the whole `let` is redundant, along with the surrounding `do`. All it needs is `else readData (addPoint l entered)`. – dfeuer Jan 08 '17 at 07:10