3
\$\begingroup\$

I've implemented a simple program to print out 4 right triangles oriented in different ways to learn Haskell. I'm sure there are more efficient ways of doing what I have done, and I just want some feedback.

All feedback related to the efficiency or quality of my code is welcome.

The four different triangle orientations are listed below

A Top Left Triangle of size 4 would look like:

****
***
**
*

A Bottom Left Triangle of size 4 would look like:

*
**
***
****

A Top Right Triangle of size 4 would look like:

****
 ***
  **
   *

A Bottom Right Triangle of size 4 would look like:

   *
  **
 ***
****

Here is my implemented solution to the problem:

module Main (main) where

import Prelude

-- MARK: Pure section --

makeTopLeftTriangle :: Char -> Int -> [String]
makeTopLeftTriangle c n
    | n < 1     = []
    | otherwise = makeTriangleRow c n : makeTopLeftTriangle c (n-1)
  where 
    makeTriangleRow :: Char -> Int -> String
    makeTriangleRow c n
        | n < 1     = []
        | otherwise = c : makeTriangleRow c (n-1)

makeBottomLeftTriangle :: Char -> Int -> [String]
makeBottomLeftTriangle c n = makeBottomLeftTriangleHelper c 1 n
  where 
      makeBottomLeftTriangleHelper :: Char -> Int -> Int -> [String]
      makeBottomLeftTriangleHelper c x n
          | x > n     = []
          | otherwise = makeTriangleRow c x : makeBottomLeftTriangleHelper c (x+1) n
        where
            makeTriangleRow :: Char -> Int -> String
            makeTriangleRow c x
                | x <= 0    = []
                | otherwise = c : makeTriangleRow c (x-1)

makeTopRightTriangle :: Char -> Int -> [String]
makeTopRightTriangle c n = [(replicate n c)]
                           ++ zipWith (++) (makeBottomLeftTriangle ' ' (n-1))
                                           (makeTopLeftTriangle c (n-1))

makeBottomRightTriangle :: Char -> Int -> [String]
makeBottomRightTriangle c n = zipWith (++) (makeTopLeftTriangle ' ' (n-1)) 
                                           (makeBottomLeftTriangle c (n-1)) 
                              ++ [(replicate n c)]

makeTriangle :: Char -> Int -> Int -> [String]
makeTriangle c triangleType = case triangleType of
    1 -> makeTopLeftTriangle c
    2 -> makeBottomLeftTriangle c
    3 -> makeTopRightTriangle c
    4 -> makeBottomRightTriangle c

-- MARK: Non-pure section --

printTriangle :: [String] -> IO ()
printTriangle []     = return ()
printTriangle (x:xs) = do 
    putStrLn x
    printTriangle xs

getTriangleType :: IO String
getTriangleType = do
    putStrLn "What type of triangle do you want to print? (1, 2, 3, or 4)"
    putStrLn "1) Top Left"
    putStrLn "2) Bottom Left"
    putStrLn "3) Top Right"
    putStrLn "4) Bottom Right"
    getLine

main :: IO ()
main = do
    i_triangleType <- getTriangleType
    i_n <- getLine

    let triangleType = read i_triangleType :: Int
    let n = read i_n :: Int

    printTriangle $ makeTriangle '*' triangleType n
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

The recursion you used is very cumbersome, and should be avoided in favour of more expressive solutions in Haskell. You figured out how to write replicate n c — why didn't you just run with that? Add some list comprehensions, and you're done!

Instead of using recursion in printTriangle to print one line at a time, you can just write putStr $ unlines.

Naming each function starting with "make…" feels a bit redundant to me.

Splitting the menu between getTriangleType and makeTriangle is awkward. You're using an integer to represent the type of triangle, and the two functions have to agree on which number represents which orientation. The most natural way to represent a type of triangle is to use the corresponding triangle-generating function itself! And I'd define a type synonym TriangleMaker for those functions.

type TriangleMaker = Char -> Int -> [String]

topLeftTriangle :: TriangleMaker
topLeftTriangle c n = [replicate i c | i <- [n, n-1 .. 1]]

bottomLeftTriangle :: TriangleMaker
bottomLeftTriangle c n = [replicate i c | i <- [1 .. n]]

topRightTriangle :: TriangleMaker
topRightTriangle c n =
  [(replicate (n - i) ' ') ++ (replicate i c) | i <- [n, n-1 .. 1]]

bottomRightTriangle :: TriangleMaker
bottomRightTriangle c n =
  [(replicate (n - i) ' ') ++ (replicate i c) | i <- [1 .. n]]

getTriangleType :: IO TriangleMaker
getTriangleType = do
  let menu = [topLeftTriangle, bottomLeftTriangle, topRightTriangle, bottomRightTriangle]
  putStr $ unlines [ 
    "What type of triangle do you want to print? (1, 2, 3, or 4)",
    "1) Top Left",
    "2) Bottom Left",
    "3) Top Right",
    "4) Bottom Right"]
  line <- getLine
  return (menu !! ((read line :: Int) - 1))

main :: IO ()
main = do
  triangle <- getTriangleType
  size <- getLine
  putStr $ unlines $ triangle '*' (read size :: Int)
\$\endgroup\$
1
  • \$\begingroup\$ I'm blown away by how much more concise your implementation is. I have a lot to improve. Thanks for helping me out! Cheers! \$\endgroup\$ Sep 27, 2018 at 13:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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