Simplifying nested pattern matching F#

Question

I am writing a simple expressions parser in F# and for each operator I want only to support a certain number of operands (e.g. two for Modulo, three for If). Here is what I have:

type Operator =
    | Modulo
    | Equals
    | If

let processOperator operands operator =
    match operator with
    | Modulo ->
        match operands with
        | [ a:string; b:string ] -> (Convert.ToInt32(a) % Convert.ToInt32(b)).ToString()
        | _ -> failwith "wrong number of operands"
    | Equals ->
        match operands with
        | [ a; b ] -> (a = b).ToString()
        | _ -> failwith "wrong operands"
    | If ->
        match operands with 
        | [ a; b; c ] -> (if Convert.ToBoolean(a) then b else c).ToString()
        | _ -> failwith "wrong operands"

I would like to get rid of or simplify the inner list matches. What is the best way to accomplish this? Should I use multiple guards ?

Gus · Answer 1 · 2014-03-17T06:34:17.073

open System

type Operator =
    | Modulo
    | Equals
    | If

let processOperator operands operator =
    match (operator, operands) with
    | Modulo, [a: string; b] -> string ((int a) % (int b))
    | Equals, [a; b] -> string (a = b)
    | If, [a; b; c]  -> if Convert.ToBoolean(a) then b else c
    | _ -> failwith "wrong number of operands"

But I would suggest to move this logic of the operands to the parser, this way you get a clean operator expression, which is more idiomatic and straight forward to process, at the end you'll have something like this:

open System

type Operator =
    | Modulo of int * int
    | Equals of int * int
    | If of bool * string * string

let processOperator = function
    | Modulo (a, b) -> string (a % b)
    | Equals (a, b) -> string (a = b)
    | If (a, b, c)  -> if a then b else c

Søren Debois · Accepted Answer · 2014-03-16T19:03:48.077

3

Fold in the operands matching:

let processOperator operands operator =
    match operator, operands with
    | Modulo, [a; b] -> (Convert.ToInt32(a) % Convert.ToInt32(b)).ToString()
    | Equals, [a; b] -> (a = b).ToString()
    | If, [ a; b; c ] -> (if Convert.ToBoolean(a) then b else c).ToString()
    | _ -> failwith "wrong number of operands"

Better yet, if you can, change the datatype to the following.

type Operator =
    | Modulo of string * string
    | Equals of string * string
    | If of string * string * string

Then in the match, you can no longer fail.

edited Mar 16 '14 at 19:03

answered Mar 16 '14 at 18:57

Søren Debois

5,598
26
48

Doesn't that just push the same problem into a different part of the code? – Nate C-K Mar 16 '14 at 18:59
You mean the datatype change. Yes. But (Gustavo and) I would argue it belongs there, along with all the conversion from integers and booleans–see Gustavo's nice answer below. – Søren Debois Mar 16 '14 at 19:10
2

Funny, we both answered at the same time pretty much the same solution, and the same recommendation, that's a design smell (a good one) ;) – Gus Mar 16 '14 at 19:17
@Gustavo & Søren : Thanks, I was was considering your first solution, but since I can change the datatypes, the second one is seems better and cleaner. I cannot mark both answers, so upvote goes to both of you, and and Answer to Søren (first). – Maciej Wozniak Mar 16 '14 at 19:31

Simplifying nested pattern matching F#

2 Answers2