0

When i compile my code in ghci, there is no problem. It can compile correctly. However, if i try to compile it in hugs, I get the error "compiled code too complex". I think the problem is due to many | conditions.

If I change it to use if/else, there is no problem. I can add if/else statements 100 times but this will be very tiresome and annoying. Rather than that, I tried to put if/else statements after 20-30 | conditions, but i cannot make | work inside if statements like the below:

f x y z     
    | cond1 = e1
    | cond2 = e2
    ...
    if (1)
    then
    | cond30 = e30
    | cond31 = e31
    ...
    else
    | cond61 = e61
    | cond62 = e62

How can I fix the code with the least effort? The complete code is on hpaste because it is longer than StackOverflow's question size limit.

Daniel Wagner
  • 145,880
  • 9
  • 220
  • 380
oiyio
  • 5,219
  • 4
  • 42
  • 54
  • Cut it into separate functions (`f'00` for `0..9`, `f'10` for `10..19`, etc.), then write `f x | 0 <= x, x < 10 = f'00 x` etc. –  Apr 06 '13 at 10:18
  • @Rhymoid ,my | statements are very complex and i cannot change them. I cannot change the general structure. I must do some trick and the code should be work correctly as it does in ghci – oiyio Apr 06 '13 at 10:23
  • 6
    Perhaps give us the actual code you have here or put your actual code in hpaste so we can see the actual problem. (You shouldn't have to write code that's this repetitive.) – AndrewC Apr 06 '13 at 10:25
  • @user1308990: what's the complex part, exactly? Is the guard complex (`| guard = value`)? If not, then splitting up the function isn't that hard. –  Apr 06 '13 at 10:28
  • i updated the post , please check it – oiyio Apr 06 '13 at 10:35
  • 1
    Are you trying to write a parser/lexer? – kennytm Apr 06 '13 at 10:36
  • I'm writing a parser . But i cannot change the above structure. I need a trick making this structure work – oiyio Apr 06 '13 at 10:39
  • 2
    Thanks for the update, but I think you're solving a problem in a needlessly complex way, and if you made clearer the purpose of your code, and gave us at least a sample of it, we could recommend a good refactoring. As it is, it's hard to know what to recommend. – AndrewC Apr 06 '13 at 10:40
  • Why do you need to compile in Hugs? – huon Apr 06 '13 at 10:41
  • Because to compile in Hugs is MUST in the course :) It works in ghci , however. @dbaupp – oiyio Apr 06 '13 at 10:43
  • 3
    Why can't you change the structure? This isn't a good way to write a parser, and it's good that Hugs isn't letting you. Are you allowed to use a parser library? If so, have a look at [this answer](http://stackoverflow.com/questions/13666150/how-to-implement-lexical-analyser-and-parser-in-haskell/13716034#13716034). If not, have a serious think about [this](http://stackoverflow.com/questions/14006707/making-a-read-instance-in-haskell/14006938#14006938) because you ought to be eating your input as you go along, not having everything in a massive interwoven set of conditions. – AndrewC Apr 06 '13 at 11:15
  • after 40 guards , i call another function(with same inputs and input types) and in that function , i check other guards :) it works – oiyio Apr 06 '13 at 12:14
  • 2
    But it's terrible! Hugs wass right. Code too complex. – AndrewC Apr 06 '13 at 12:35
  • 2
    If you have many lines 420 characters long that differ from eachother by precisely 1 character, then that is a sure sign that you're doing something horribly, horribly wrong. You NEED to start abstracting those repeated patterns into separate functions which you can call repeatedly. Once you've done that you may see more, higher level repetitions you can abstract, and in the end you may see a new and better approach to your problem. – Neil Forrester Apr 06 '13 at 13:38
  • @user1308990 Are you supposed to do the operations in the order they appear, so that `2+5*10=70` or using the precedence according to the normal ones from arithmetic, where * and / are prioritised over + and -, so that `2+5*10=52`? – AndrewC Apr 06 '13 at 13:45

2 Answers2

9

Avoiding repetitive guards

Firstly, you can rewrite

function input 
   | this && that && third thing && something else = ...   -- you only actually needed brackets for (head xs) 
   | this && that && third thing && something different = ....
   | this && that && a change && ...  
...
   | notthis && ....

with

function input | this = function2 input'
               | notthis = function4 input'

function2 input | that = function3 input''
                | notthat = ...

That should simplify your 200 lines of copo code down, but it's still the wrong approach.

Use a function to deal with the same problem just once, not every time

The 4 cases for dealing with operations that you deal with time after time could be replaced with one function, perhaps like:

operation :: Num a => Char -> a -> a -> a
operation x = case x of
   '+' -> (+)
   '-' -> (-)
   '*' -> (*)
   '/' -> (/)
   _   -> error ("operation: expected an operation (+-*/) but got " ++ [c])

Use list functions instead of testing characters one at a time

You should use some standard functions to help reduce all the single character checks into just grabbing as much number as is there. takeWhile :: (a -> Bool) -> [a] -> [a], so

takeWhile isDigit "354*243" = "354"
takeWhile isDigit "+245" = ""

and there's the corresponding dropWhile:

dropWhile isDigit "354*1111" = "*1111"
dropWhile isDigit "*1111" = "*1111"

So the most dramatic shortening of your code would be to start copo with

copo xs = let 
  numText = takeWhile isDigit xs
  theRest = droWhile isDigit xs 
  num = read numText
  ....
    in answer....

but there's a shortcut if you want both takeWhile and dropWhile, called span, because span p xs == (takeWhile p xs, dropWhile p xs)

copo xs = let 
  (numText,theRest) = span isDigit xs
  num = read numText
  ....
    in answer....

Use recursion instead of repeating code

You deal with 234 then 234*56 then 234*56/23 then ....

You could replace this with a recursive call to copo, or produce a tree. This depends on whether you're supposed to obey the normal operator precedence (* or / before + or -) or not.

AndrewC
  • 32,300
  • 7
  • 79
  • 115
0

If you insist on guards, instead of

foo a b c d
    | cond1, cond2, cond3 = ...
    | cond1, cond2, cond4 = ...
    | cond5, cond6, cond7 = ...
    | cond5, cond6, cond8 = ...

write

foo a b c d
    | cond1, cond2 = case () of
        () | cond3 = ...
           | cond4 = ...
    | cond5, cond6 = case () of
        () | cond7 = ...
           | cond8 = ...
Ingo
  • 36,037
  • 5
  • 53
  • 100