3
\$\begingroup\$

I implemented an algorithm that gives me a list of points that should be drawn in order the get a line between two points. Please note: it works only in a coordinate system whose origin (0,0) is in the upper left corner.

I'd like to get some review on my Haskell code, could it be improved in terms of the language constructs used? Could it be more readable, more straight forward?

module DrawLine where

data Point = Point Float Float deriving (Show)

getRange :: Int -> Int -> [Int]
getRange a b
    | a <= b = [a..b]
    | otherwise = [b..a]
{-|
 - Get the slope of a line described by the two points received as arguments


-}
getSlope :: Point -> Point -> Float
getSlope (Point x1 y1) (Point x2 y2) = (y2-y1)/(x2-x1)

{-|
 - Get a list of points where pixels should be filled to draw a line
 - (Inspired from Bresenham's line algorithm)
 - Takes two Point arguments representing the end points of the line, the points
 - will be located on a line between these two points
 -}
getLinePoints :: Point -> Point -> [Point]
getLinePoints f@(Point fx fy) l@(Point lx ly)
    -- vertical line
    | fx == lx = [(Point fx (fromIntegral y)) | y <- getRange (round fy) (round ly)]
    | otherwise =
        -- generate points by mapping on the "longer side" of the line
        if m > 1.0 || m < -1.0 then
            -- the rise if higher than the run
            map (\y ->
                    (Point
                        ((1/m) * ((fromIntegral y) - fy) + fx)
                        (fromIntegral y)
                    )
                )
                (getRange (round fy) (round ly))
        else
            map (\x ->
                    (Point
                        (fromIntegral x)
                        (m * ((fromIntegral x) - fx) + fy)
                    )
                )
                (getRange (round fx) (round lx))
        where
            m = getSlope f l 
\$\endgroup\$
4
  • 1
    \$\begingroup\$ where m… what? \$\endgroup\$ – 200_success Dec 31 '13 at 10:22
  • \$\begingroup\$ updated: where m = getSlope f l, sorry for that, copy&paste error \$\endgroup\$ – Paul Dec 31 '13 at 10:26
  • 1
    \$\begingroup\$ As a Haskell novice, I'd say that it looks fine. \$\endgroup\$ – 200_success Jan 1 '14 at 6:03
  • \$\begingroup\$ Thank you. What if I'd like to advance a little bit? What other language features/concepts could be used in this context? \$\endgroup\$ – Paul Jan 1 '14 at 11:14
2
\$\begingroup\$

Thoughts.

Define a Line type and rename getLinePoints to mkLine. A list of points may represent anything but a Line [Points] implies the Points within the line. Where possible let the type system work for you. Of course you could get cute and have Line (m -> x -> b -> [Points]) or List Point Point.

You could define Eq and Ord instances for the Point and maybe Line types.

What if getSlope is passed a vertical line? Should it perhaps use Maybe?

I'd move the non-vertical line support into its own function. Perhaps add a horizontal line function as well.

Why use a list comprehension for the vertical one and map for the other type of line? It can all be done with a comprehension.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you! I'll definitely take a closer look on what you said and try to improve things. \$\endgroup\$ – Paul Mar 26 '14 at 6:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.