Assistance making a function more readable and elegant.

Erik Zuurbier F.S.A.Zuurbier@inter.nl.net
Wed, 26 Nov 1997 01:22:11 +0100


Nick,
I have used the Parser functions of the Clean book myself. With
that background I started to read your formData function before
reading your explanations.

I seriously think your program reads like a poem. I understood
each phrase on first reading. There is no need to change anything
at all from that point of view. Even the comments you added seem
ill advised to me: it would only confuse people. All of this is
due both to your chosing meaningful names isHexDigit and hexdigtoInt,
and to Clean's expressiveness.

If you are really bothered by the program being monolithic, you
can lift all the items after 'where' to the top level. This makes
the program more fine-grained but it destroys some of the scoping
introduced by the 'where's.

If you need to add comments, do not just literally translate into
English what you just wrote down in Clean: English (or any natural
language) is a bad choice for that and it denies what Clean is all
about: a specification language. Instead, add complementary comments:
describe why you wrote the parser in the first place.

If the text that you are parsing has a syntax that was specially
designed to be parsed, it may be possible to replace <*> by <!*>
and <|> by <!>. That speeds up the parser by ignoring unlikely
interpretations.

The last line of your program  contains:
 otherwise = 9 + toInt (toLower c) - toInt 'a' + 1
You can of course replace 9 - toInt 'a' + 1 by a constant
that you calculate once.

Regards Erik Zuurbier



>formData :: Parser Char [Char]
>formData = <*> (valid <|> space <|> special)
> where
>    valid = satisfy (\c -> not (isMember c ['&', '=', '+', '%']) && isAscii
>c)
>    space = symbol '+' <@ (\_ -> ' ')
>    special = symbol '%' &> satisfy isHexDigit <&> satisfy isHexDigit <@
>(\(a, b) -> toChar (hexdigtoInt a * 16 + hexdigtoInt b))
>    where
>        isHexDigit c = isDigit c || (isMember (toLower c) ['a'..'f'])
>        hexdigtoInt c
>            |    isDigit c = digtoInt c
>            |    otherwise = 9 + toInt (toLower c) - toInt 'a' + 1
>