Compute the perimeter of a polygon This is taken from HackerRank, which I hope is okay.

I've been playing around with Haskell and I haven't really been able to find anything that really goes beyond toy examples in terms of how to write a readable, well-structured program.  Several parts of this seem messy but I guess I don't really know any better.  Anyway, here's my code, which works (or at least passes all the unit tests supplied at HackerRank).

import Control.Monad

-- Given a list, return the pairs of consecutive elements in the list
consec :: [a] -&gt; [(a,a)]
consec xs = zip xs (drop 1 (cycle xs))

-- Given a list of two elements, return a 2-tuple of the elements
make2Tuple :: [a] -&gt; (a, a)
make2Tuple [x, y] = (x, y)

-- Apply a given function to the first element of a list
applyToFirst :: (a -&gt; a) -&gt; [a] -&gt; [a]
applyToFirst _ [] = []
applyToFirst f (x:xs) = (f x):xs

-- Split a list at a specified delimiter
split :: Eq a =&gt; a -&gt; [a] -&gt; [[a]]
split _ [] = [[]]
split y (x:xs)
  | (x == y) = []:(split y xs)
  | (x /= y) = applyToFirst (x:) (split y xs)

-- Convert a string to a list of integers
stringToInts :: [Char] -&gt; [Int]
stringToInts = map (read :: [Char] -&gt; Int) . (split ' ')

-- Action that reads in a 2-tuple of integers
getInt2Tuple :: IO (Int, Int)
getInt2Tuple = fmap (make2Tuple . stringToInts) getLine

euclideanDistance :: ((Int, Int), (Int, Int)) -&gt; Double
euclideanDistance ((x1, y1), (x2, y2)) = sqrt(fromIntegral((y2-y1)*(y2-y1) + (x2-x1)*(x2-x1)))

computePerimeter :: [(Int, Int)] -&gt; Double
computePerimeter xs = sum (map euclideanDistance (consec xs))

main :: IO ()
main = (fmap computePerimeter (fmap (read :: [Char] -&gt; Int) getLine &gt;&gt;= (\n -&gt; replicateM n getInt2Tuple))) &gt;&gt;= print


Sample input

4
(0, 0)
(0, 1)
(1, 1)
(1, 0)


Sample output

4


Explanation

The input says that this polygon has four vertices, and then lists the vertices of a unit square.  The perimeter of a unit square is 4 (one unit per edge times four edges), which is what's output.
consec will wrap around and pair the last element of your list as consecutive with the first. This isn't documented in the comment and IMO a little bit problematic. While it lends itself well to the problem you're solving, I'd try to change the implementation of consec a little and being explicit about the cycle property in the name.

Consider:

consec :: [a] -&gt; [(a,a)]
consec [] = []
consec [x] = []
consec (x:xs@(y:_)) = (x,y):consec xs


This implementation has the benefit of being more explicit over zip xs (drop 1 $ cycle xs). It can be trivially extended to a consecCycle like so:

consecCycle :: [a] -&gt; [(a,a)]
consecCycle [] = []
consecCycle [y] = []
consecCycle xs@(x:_) = consec (xs ++ [x])


For what it's worth, this is basically a stylistic call. I'm more prone to overspecifying my functions, avoiding pointfree style. I guess I just dislike oneliners...



make2Tuple is a dangerous one. You're explicitly forcing the function to be called with a list of only two elements by accepting a runtime error of "Non-exhaustive patterns in function make2Tuple". 

I'd consider this bad practice. To totally define the function you could consider using a Maybe like so:

make2Tuple :: [a] -&gt; Maybe (a,a)
make2Tuple [x,y] = Just (x,y)
make2Tuple _ = Nothing


this instantly communicates that the function is not totally defined and it avoids a runtime error.



applyToFirst is cleanly written and it clearly communicates intent. Yet I dislike the function because you're using it as a helper for something that's cleaner to implement without it.

split is a weird name for that function, I'd instead use splitWhere or splitIf and supply a predicate, which leads to the following typesignature:

splitWhere :: (a -&gt; Bool) -&gt; [a] -&gt; [[a]]


which can then be implemented as follows:

splitWhere :: (a -&gt; Bool) -&gt; [a] -&gt; [[a]]
splitWhere _ [] = [[]]
splitWhere p ls = go [] ls
    where 
        go accum (x:xs) = if p x then accum:go [x] xs else go (accum++[x]) xs
        go accum [] = [accum]


The advantage of this implementation is in the use of an "accumulator" instead of the applyToFirst you're using to achieve the same functionality.

It's also a tad easier to grasp, IMO, but YMMV



stringToInts is map read . words. Note that without an explicit type-signature GHCI can not infer the correct type, since read is not specified out enough.

Unfortunately this makes the work put into applyToFirst and split unnecessary :(



I'd simplify euclideanDistance's pattern by splitting up the tuple of tuples into separate parameters. To keep in line with how it's invoked you can then adjust computePerimeter to uncurry it like so:

computePerimeter xs = sum $ map (uncurry euclideanDistance) (consec xs)




Finally I find it hard to keep track of how your main works. It's probably easier to rewrite this as a do like so:

main = do
    vertices &lt;- getLine
    polygon &lt;- replicateM (read vertices) getInt2Tuple
    print $ computePerimeter (map fromJust $ filter isJust polygon)


This is IMO significantly easier to follow than your oneliner (and it also avoids all the fmap

Zeta noted that the last line can be shortened (and secured a bit) to:

print $ computePerimeter $ catMaybes polygon


Note that I've added the following imports and changed the getInt2Tuple signature to be an IO (Maybe (Int, Int)) instead:

import Control.Monad(replicateM)
import Data.Maybe(fromJust, isJust)