CODEWITHSUNDEEP
haskell
Alaya
Alaya

Reputation: 3387

why would I get a "xx not in scope error" for this haskell code?

I write such a code, but get an error when compiling saying that "xx is not in scope". 

test x =
    let xx = 2 * x
     in result 
       where result = replicate xx 3

I know I can fix it by using in replicate xx 3, however, code above is just a demo, the real code I am dealing with is like below:

nthElement :: (Ord b)=>(a->b)->Int->[a]->[a]
nthElement _ _ []  = []
nthElement _ _ [x] = [x]
nthElement op k vals@(x:xs)
    | k > (length vals) = vals
    | otherwise = let left    = [p | p<-vals, (op p) < (op x)]
                      midd    = [p | p<-vals, (op p) == (op x)]
                      right   = [p | p<-vals, (op p) > (op x)]
                      leftLen = length left
                      middLen = length midd
                   in result where result | leftLen >= k             = (nthElement op k left) ++ midd ++ right
                                          | (leftLen + middLen) >= k = left ++ midd ++ right
                                          | otherwise                = left ++ midd ++ nthElement op (k-middLen-leftLen) right

It seems that if I don't use the where clause, I have to use deeply nested if like this:

nthElement :: (Ord b)=>(a->b)->Int->[a]->[a]
nthElement _ _ []  = []
nthElement _ _ [x] = [x]
nthElement op k vals@(x:xs)
    | k > (length vals) = vals
    | otherwise = let left   = [p | p<-vals, (op p) < (op x)]
                      midd   = [p | p<-vals, (op p) == (op x)]
                      right  = [p | p<-vals, (op p) > (op x)]
                      leftLen = length left
                      middLen = length midd
                   in if leftLen >= k 
                         then (nthElement op k left) ++ midd ++ right
                         else if (leftLen + middLen) >= k 
                                then left ++ midd ++ right
                                else left ++ midd ++ nthElement op (k-middLen-leftLen) right

So, how could I change my code to fix the compiling bug as well as avoide using nested if?

Upvotes: 1

Views: 114

Answers (2)

bheklilr
bheklilr

Reputation: 54058

You should think of this code more as

test x = {
    let {
        xx = 2 * x
    } in {
        result
    }
} where {
    result = replicate xx 3
}

instead of

test x = {
    let {
        xx = 2 * x
    } in {
        result where {
            result = replicate xx 3
        }
    }
}

The where clause covers the entire definition of the function body and can only use names defined outside the function body (which arguments to test and test itself). The best way to fix this would be to move all definitions to the let or to the where. For your case, you'll probably want to move them all into the let:

test x =
    let xx = 2 * x
        result = replicate xx 3
    in result

Or for your actual use case:

nthElement :: (Ord b) => (a -> b) -> Int -> [a] -> [a]
nthElement _ _ []  = []
nthElement _ _ [x] = [x]
nthElement op k vals@(x:xs)
    | k > (length vals) = vals
    | otherwise = let left    = [p | p<-vals, (op p) < (op x)]
                      midd    = [p | p<-vals, (op p) == (op x)]
                      right   = [p | p<-vals, (op p) > (op x)]
                      leftLen = length left
                      middLen = length midd
                      result | leftLen >= k             = (nthElement op k left) ++ midd ++ right
                             | (leftLen + middLen) >= k = left ++ midd ++ right
                             | otherwise                = left ++ midd ++ nthElement op (k-middLen-leftLen) right
                   in result

But, as this marches right off the side of the page, I would refactor it a bit to just use a single guard and a where:

nthElement :: (Ord b) => (a -> b) -> Int -> [a] -> [a]
nthElement _ _ []  = []
nthElement _ _ [x] = [x]
nthElement op k vals@(x:xs)
    | k > length vals  = vals
    | k <= leftLen     = nth k left ++ midd ++        right
    | k <= leftMiddLen =       left ++ midd ++        right
    | otherwise        =       left ++ midd ++ nth kR right
    where
        opx         = op x
        left        = [p | p <- vals, op p <  opx]
        midd        = [p | p <- vals, op p == opx]
        right       = [p | p <- vals, op p >  opx]
        leftLen     = length left
        middLen     = length midd
        leftMiddLen = leftLen + middLen
        nth         = nthElement op
        kR          = k - leftMiddLen

98% of this is just stylistic, you may not like it this way, but I find it a lot easier to read. In particuarly, I would say that the 2% that isn't just style is collapsing the guards down to a single level, it makes your intentions much more clear. Since Haskell is lazy you also don't have to worry about computing anything until the value is actually used.

Upvotes: 3

bwroga
bwroga

Reputation: 5449

nthElement :: (Ord b)=>(a->b)->Int->[a]->[a]
nthElement _ _ []  = []
nthElement _ _ [x] = [x]
nthElement op k vals@(x:xs)
    | k > (length vals) = vals
    | otherwise = let left    = [p | p<-vals, (op p) < (op x)]
                      midd    = [p | p<-vals, (op p) == (op x)]
                      right   = [p | p<-vals, (op p) > (op x)]
                      leftLen = length left
                      middLen = length midd
                      result | leftLen >= k             = (nthElement op k left) ++ midd ++ right
                             | (leftLen + middLen) >= k = left ++ midd ++ right
                             | otherwise                = left ++ midd ++ nthElement op (k-middLen-leftLen) right
                   in result

Upvotes: 1

Related Questions