Redundant pattern matching warning

user_6684107 0
Viewed 451

I am writing a piece of code that is a function returning a list of pixels which is a special data type I have defined which is of the following:

data Pixel = Pixel {
                    color :: Float,
                    elevation :: Float,
                    distance: Float,
                    visited :: Bool
                   } deriving (Show)

I have this function that is going to go through two files and put them together into a list of pixels. The files contain the color and elevation respectively so I proceed to get this code output:

insertPixels :: [String] -> [String] -> [Pixel]
insertPixels _ _ = []
insertPixels (x:xs) (y:ys) = [Pixel (read x) (read y) 99999999 False] ++ insertPixels xs ys

When I attempt to compile this or load it into the interpreter, I receive this error:

lab1.hs:33:1: warning: [-Woverlapping-patterns]
Pattern match is redundant
In an equation for ‘insertPixels’:
    insertPixels (x : xs) (y : ys) = ...
|
| insertPixels (x:xs) (y:ys) = [Pixel (read x) (read y) 99999999 False] ++ insertPixels xs ys
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I don't really understand what this means or what I am doing wrong.

pattern-matching haskell
2 Answers

Just switch round the order of your 2 lines, like this:

insertPixels :: [String] -> [String] -> [Pixel]
insertPixels (x:xs) (y:ys) = [Pixel (read x) (read y) 99999999 False] ++ insertPixels xs ys
insertPixels _ _ = []

The problem is that insertPixels _ _ matches everything, and since patterns are evaluated from top down, stopping at the first match, all possible inputs would result in the empty list - clearly not what you intended. As corrected, the recursive case will take effect whenever both input lists are nonempty, and as soon as one is empty, it will fall through to the "catch-all" case and result in the empty list.

As a side note, it's not particularly idiomatic to use the (++) operator to add a single element to the start of a list. You can use the (:) ("cons") operator instead:

insertPixels :: [String] -> [String] -> [Pixel]
insertPixels (x:xs) (y:ys) = (Pixel (read x) (read y) 99999999 False) : insertPixels xs ys
insertPixels _ _ = []
user_84750540

The underscore _ is a wildcard, this thus means that it matches anything: empty lists, non-empty lists, etc.

This thus means that the first line, will "fire", regardless what values you pass to insertPixels. There is no input pattern for which the second line will "fire", hence the warning.

We can use pattern matching on empty lists, like:

insertPixels :: [String] -> [String] -> [Pixel]
insertPixels [] _ = []
insertPixels _ [] = []
insertPixels (x:xs) (y:ys) = [Pixel (read x) (read y) 99999999 False] ++ insertPixels xs ys

What you here describe however is a well-known pattern. A zipWith :: (a -> b -> c) -> [a] -> [b] -> [c]. We can thus define the insertPixels as:

import Data.Function(on)

insertPixels :: [String] -> [String] -> [Pixel]
insertPixels = zipWith (\x y -> on Pixel read x y 99999999 False)

I would advice however to remove the read out of the insertPixels function, and thus work with two lists of Floats. If you want to use read, then you can do that at the caller level.

user_675790
Related