CODEWITHSUNDEEP
haskellrefactoring
Zazaeil
Zazaeil

Reputation: 4119

Best way to refactor such a dummy code with help of Haskell's standard libraries?

I am currently working with quite dirty project written in Haskell. It contains lots of code like:

try_parse_parameters (p:ps) kvps options = 
        let maybeKvp = try_parse_kvp p 
        in
          if isJust maybeKvp 
            then try_parse_parameters ps ((fromJust maybeKvp) : kvps) options
            else 
              let maybeOption = try_parse_option p
              in 
                if isJust maybeOption
                  then try_parse_parameters ps kvps ((fromJust maybeOption) : options)
                  else try_parse_parameters ps kvps options

Thus, my questions is: is there some standard method(s) to deal with such a situation?

Upvotes: 2

Views: 89

Answers (2)

Euge
Euge

Reputation: 749

You can also handle more than one case with pattern matching, avoiding the pyramid of doom.

try_parse_parameters (p:ps) kvps options = 
    case (try_parse_kvp p, try_parse_option p) of
      (Just k, _)       -> try_parse_parameters ps (k : kvps) options
      (Nothing, Just o) -> try_parse_parameters ps kvps (o : options)
      (Nothing, Nothing)-> try_parse_parameters ps kvps options

Upvotes: 5

jberryman
jberryman

Reputation: 16645

Yes, what you're looking for is called pattern matching, and you can't really write haskell without it. Here's the most straightforward adaptation of your code:

try_parse_parameters (p:ps) kvps options = 
    case try_parse_kvp p of
      Just kvp -> try_parse_parameters ps (kvp : kvps) options
      Nothing -> 
        case try_parse_option p of
          Just o -> try_parse_parameters ps kvps (o : options)
          Nothing -> try_parse_parameters ps kvps options

From here there are a lot of things you might improve, now or later as you learn more haskell:

  • I would really suggest using CamelCase, which is standard but also more readable because you can easily distinguish functions from their arguments
  • you're doing a fold over the list. You could use foldr to factor out the recursion and make it more clear to people reading your code what it does (and what it can't do; i.e. making it easier to reason about)
  • add a type signature, handle the empty list case, maybe other issues
  • You might find that the Alternative class is helpful, or using a monadic fold perhaps if you want the entire computation to fail if any of the parses fail completely

Upvotes: 4

Related Questions