CODEWITHSUNDEEP
haskell
Vitaly Olegovitch
Vitaly Olegovitch

Reputation: 3547

Wrong output of simple Haskell exercise solution

I am trying to solve this exercise:

You are given a sequence of N balls in 4 colors: red, green, yellow and blue. The sequence is full of colors if and only if all of the following conditions are true:

  • There are as many red balls as green balls.
  • There are as many yellow balls as blue balls.
  • Difference between the number of red balls and green balls in every prefix of the sequence is at most 1.
  • Difference between the number of yellow balls and blue balls in every prefix of the sequence is at most 1.

Your task is to write a program, which for a given sequence prints True if it is full of colors, otherwise it prints False.

My solution so far is:

module Main where

import Control.Monad

main = do
  s <- readLn :: IO Int
  elements <- replicateM s getLine
  let result = map checkColours elements
  mapM (putStrLn . show) result

checkColours :: String -> Bool
checkColours string = isFullOfColors 0 0 0 0 string

isFullOfColors :: Int -> Int -> Int -> Int -> String -> Bool
isFullOfColors red green yellow blue string
  | (abs (red - green)) > 1 = False
  | (abs (yellow - blue)) > 1 = False
  | (string == []) = if (red /= yellow) || (green /= blue) then True else False
  | (head string == 'R' ) = isFullOfColors (red + 1) green yellow blue (tail string)
  | (head string == 'G' ) = isFullOfColors red  (green + 1) yellow blue (tail string)
  | (head string == 'Y' ) = isFullOfColors red green (yellow + 1) blue (tail string)
  | (head string == 'B' ) = isFullOfColors red green yellow  (blue + 1) (tail string)

But it fails on the input "RYBG", returning False instead of True.

What am I doing wrong?

Upvotes: 0

Views: 121

Answers (1)

sshine
sshine

Reputation: 16105

Besides C. Quilley's comments, here is some general advice and a suggestion for structuring your function differently. The guard string == [] is better written as null string, since the built-in function null will make this decision with a single pattern match, rather than having to rely on the list of elements being comparable (which it only is when the elements of the list themselves are comparable). The pattern if ... then True else False can be shortened to ..., since this is already a boolean with the same value. Generally, try and use pattern matching!

I cannot see where you derive (red /= yellow) || (green /= blue) from. When does the number of red balls and yellow balls have constraints in common?

Instead of a String, you may want to create a data type that reflects balls:

data Ball = Red | Green | Yellow | Blue
          deriving Eq

You may want them to be displayed just the same as before:

instance Show Ball where
    show Red = "R"
    show Green = "G"
    show Yellow = "Y"
    show Blue = "B"

And you may want to embed the helper function inside the main function:

isFullOfColors :: [Ball] -> Bool
isFullOfColors = go (0, 0, 0, 0)
  where
    go :: (Int, Int, Int, Int) -> [Ball] -> Bool
    go (r,g,y,b) (Red:balls) = ...
    go (r,g,y,b) (Green:balls) = ...
    go (r,g,y,b) (Yellow:balls) = ...
    go (r,g,y,b) (Blue:balls) = ...
    go (r,g,y,b) [] = ...

As for hints on the logical part: These can be expressed as comparisons between r, g, y and b in combination with recursive calls to balls.

Upvotes: 3

Related Questions