Haskell compositor

This is a concept for a compositor. It would help me if you could review how well I expressed my ideas into the functional language.

I first divide everything in an areas that each window covers entirely or not:

Then I can walk the areas and render each corresponding window.

The actual composing of the vectors is a type hole right now because that's not really what this is about.

```
module SceneGraph where
import qualified Data.Vector as V
import Codec.Picture
import Data.Foldable
import Data.List
import Control.Applicative
import Data.Colour
import Data.Colour.SRGB
import Data.Colour.Names

data Point = Point { xC, yC :: Int}
--[upleft, downright)! upleft inclusive, downright exclusive
data Rectangle = Rectangle { upLeft, downRight :: Point}
--[upleft, downright]! inclusive
data Area = Area { upLeftA, downRightA :: Point}
data Window = Window
{ zIndex :: Int,
area :: Rectangle,
render :: V.Vector PixelRGB8 -> V.Vector PixelRGB8
}

getAllPoints :: [[Window]] -> [Point]
getAllPoints = foldMap (\z -> [upLeft (area z), downRight(area z)]) . fold

getXYCoordinatesSorted :: [Point] -> ([Int], [Int])
getXYCoordinatesSorted = sortPoints . foldr (\z y -> (xC z : fst y, yC z : snd y)) ([], [])

sortPoints :: ([Int], [Int]) -> ([Int], [Int])
sortPoints z = ((sort . nub . fst) z, (sort . nub . snd) z)

getBounds :: ([Int], [Int]) -> ([(Int, Int)], [(Int, Int)])
getBounds z = ((constructBounds . fst) z, (constructBounds . snd) z)

constructBounds :: [Int] -> [(Int, Int)]
constructBounds z = map (\i -> (fst i, snd i - 1)) $ zip z (tail z) ++ [(last z, last z + 1)]

boundsToAreas :: ([(Int, Int)], [(Int, Int)]) -> [Area]
boundsToAreas (xAxis, yAxis) = liftA2 boundToArea xAxis yAxis

boundToArea :: (Int, Int) -> (Int, Int) -> Area
boundToArea (xUp, xDown) (yLeft, yRight) = Area (Point xUp yLeft) (Point xDown yRight)

targetedWindows :: [[Window]] -> Point -> [Window]
targetedWindow

There are several ways in which "well expressed into the functional language" can be understood.

Idiom

You make a large use of maps, folds, list, tuples etc. and privilege small functions. That’s idiomatic enough for me.

If you want to write even more idiomatic code, you can use HLint and/or pointfree.io.

For example, pointfree.io suggest:

boundsToAreas (xAxis, yAxis) = liftA2 boundToArea xAxis yAxis

Can be written:

boundsToAreas = uncurry (liftA2 boundToArea)

HLint suggest:

foldr composeRenderings
    (createBackground (constructBackgroundArea areas))
    (map (`renderWindow` window) areas)

Can be written:

foldr (composeRenderings . (`renderWindow` window))
    (createBackground (constructBackgroundArea areas))
    areas

Types

4 types

4 different types are defined in your module (perhaps you put them in this code to make it easier to understand). You should put them in separate modules. It will allow you to create dedicated functions for them.

Confusion

There is a confusion between Area and Rectangle:

• Rectangle does not include the bottom right point,

• Area does include the bottom right point.

Therefore, why is the Window’s area a Rectangle ?

A Rectangle and an Area are the same thing, it’s how you use them that makes a difference. How to use them is the functions’ job. You could differentiate them with functions like isInsideInclusive and isInsideExclusive.

Intermediate types

You make use of complex yet generic types like ([Int], [Int]), (Int, Int), [(Int, Int)] or even ([(Int, Int)], [(Int, Int)]) though the functions using them are not generic.

You can use type alias to make their intent clearer. For example:

type VerticalStripe = (Int, Int)
type HorizontalStripe = (Int, Int)

boundToArea :: VerticalStripe -> HorizontalStripe -> Area
boundsToAreas :: ([VerticalStripe], [HorizontalStrip]) -> [Area]

Putting the functions using them into another module would be a good idea.

Readability

You should avoid making too long lines of code and better format your code.

For example, this formatting is hard to read:

isPointInsideRectangle :: Rectangle -> Point -> Bool
isPointInsideRectangle rect point = (xC point >= xC (upLeft rect) && yC point = xC (upLeft rect) && yC point <= yC (upLeft rect))

While the following formatting is easier to read and tells you something is wrong:

isPointInsideRectangle :: Rectangle -> Point -> Bool
isPointInsideRectangle rect point =
       (xC point >= xC (upLeft rect) && yC point = xC (upLeft rect) && yC point <= yC (upLeft rect))

Modularity

In your module, the calling graph is a five levels tree:

For example, you have the following chain:

composeWindows → renderWindows → renderWindow → targetedWindows → isPointInsideRectangle

That’s too much for one module. Plus, what is the direct link between composeWindows and isPointInsideRectangle ?

The composeWindows function makes it clear that it does 2 different things:

composeWindows windows =
    renderWindows (( boundsToAreas
                   . getBounds
                   . getXYCoordinatesSorted
                   . getAllPoints
                   ) windows
                  )
                  windows

In renderWindows first parameter, it calculates the areas. This computation is totally unrelated to a Window or its rendering, this is just a partition of a Rectangle.

Code cleaning

If a function is the unique caller of another function, it can generally include this function into its body via a where statement.

boundsToAreas :: ([(Int, Int)], [(Int, Int)]) -> [Area]
boundsToAreas = uncurry (liftA2 boundToArea)

boundToArea :: (Int, Int) -> (Int, Int) -> Area
boundToArea (xUp, xDown) (yLeft, yRight) =
    Area (Point xUp yLeft) (Point xDown yRight)

can be written:

boundsToAreas :: ([(Int, Int)], [(Int, Int)]) -> [Area]
boundsToAreas = uncurry (liftA2 boundToArea)
    where boundToArea (xUp, xDown) (yLeft, yRight) =
              Area (Point xUp yLeft) (Point xDown yRight)

Doing so make it clear the boundToArea function is not used anywhere else or that it does not need to be exported by your module.

This will also avoid to have all the module functions at the same level which can make the code less readable.

The lengthArea and heightArea functions are not used at all.

StackExchange Code Review Q#113983, answer score: 2