7

I have a C# method that looks a bit like this:

bool Eval() {
  // do some work
  if (conditionA) {
     // do some work
     if (conditionB) {
       // do some work
       if (conditionC) {
         // do some work
         return true;
       }
     }
  }
  return false;
}

In F# this ends up looking quite a bit uglier because of the mandatory else branches:

let eval() =
  // do some work
  if conditionA then
    // do some work
    if conditionB then
      // do some work
      if conditionC then
        // do some work
        true
      else
        false
    else
      false
  else
    false

What would be a cleaner way of writing this in F#?

ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
Asik
  • 21,506
  • 6
  • 72
  • 131
  • 2
    there is definitely a cleaner way to write this in C#. Inverse your conditions and return false. Not sure if it helps with F# though. – Ilia G Aug 15 '12 at 20:26
  • Wow--lots of good answers on this question. – Onorio Catenacci Aug 16 '12 at 14:07
  • @Onorio - I disagree; there are lots of "interesting" answers, but in my opinion, most of the answers add complexity, and I thought our goal as software engineers was to reduce it. – Brian Aug 16 '12 at 16:05

6 Answers6

13
module Condition =
  type ConditionBuilder() =
    member x.Bind(v, f) = if v then f() else false
    member x.Return(v) = v
  let condition = ConditionBuilder()

open Condition

let eval() =
  condition {
    // do some work
    do! conditionA
    // do some work
    do! conditionB
    // do some work
    do! conditionC
    return true
  }
Daniel
  • 47,404
  • 11
  • 101
  • 179
  • 3
    @Brian I believe your comment is a bit out of line. People may decide for themselves what is and what is not readable, and what is and what is not a good answer. Ethics aside, readability is in the eye of the beholder - different people from different backgrounds have easier or tougher time reading different coding styles. – Ramon Snir Aug 16 '12 at 10:09
  • 2
    My opinion is that this is an awful answer. I think the vast majority of programmers would prefer to see the original code, or code like mine with basic if-thens and a single local mutable variable, than this code which uses esoteric language features to introduce a new control abstraction in order to... I dunno what, be more elegant? Save one line? I just don't get it. I guess there are multiple sub-cultures in the FP community, and I am completely out of touch with this one. – Brian Aug 16 '12 at 16:02
  • 5
    @Brian: FWIW, I am not part of the die-hard FP "sub-culture" either. But, you could argue this is an advertisement for F# as it's intended to be: 1) interesting/fun, 2) demonstrate F#'s rather diverse toolkit for solving problems, and 3) drive interest in concepts that are worth learning and may be new to some programmers (monads). Is that _awful_? – Daniel Aug 16 '12 at 16:08
  • 2
    I guess my main fear is, I interpret the question as an F# newbie syntax question (OP does not know 'if statement'), and so I don't think a bunch of answers that involve monads is the best way to introduce the person to the language and community. It is always a challenge on SO not knowing the context/background of the original poster, as well as people who find/read the question later, to know what 'kind' of answer will be most useful to them. – Brian Aug 16 '12 at 16:42
  • Given the variety of answers here, I assume he will understand some (and use them) and be intrigued by others (driving interest/learning) – Daniel Aug 16 '12 at 17:10
  • 1
    @Brian - Of course a newbie will look at this and do a double take but anyone who is dedicated to programming will try to understand what exactly is going on and be a better programmer for it. If we always write code for the lowest common denominator no progress would be made. That being said I think all syntax variations should be shown and the OP can choose what feels natural to them. The power and flexibility of the syntax is one of the greatest features of F#. – ChaosPandion Aug 16 '12 at 20:08
  • 5
    @Brian - I'm sure you are aware of this but many of the feature we have come to love in C# and F# were considered quite "Esoteric", "Elitist", "Generally Useless" to the programming masses for years upon years. That is until someone with foresight felt that maybe the masses weren't that stupid and could figure out what is going on. – ChaosPandion Aug 16 '12 at 20:23
9

As mentioned in the comments, you could inverse the condition. This simplifies the C# code, because you can write:

if (!conditionA) return false;
// do some work

Although F# does not have imperative returns (if you want to return, you need both true and false branches), it actually simplifies this code a bit too, because you can write:

let eval() = 
  // do some work 
  if not conditionA then false else
  // do some work 
  if not conditionB then false else
  // do some work 
  if not conditionC then false else
    // do some work 
    true 

You still have to write false multiple times, but at least you don't have to indent your code too far. There is an unlimited number of complex solutions, but this is probably the simplest option. As for more complex solution, you could use an F# computation expression that allows using imperative-style returns. This is similar to Daniel's computation, but a bit more powerful.

Tomas Petricek
  • 240,744
  • 19
  • 378
  • 553
6

Well, since 'do some work' is already imperative (presumably), then I think that

let eval() =
    let mutable result = false
    ... // ifs
        result <- true
    ... // no more elses
    result

is shorter and reasonable. (In other words, else is only mandatory for if expressions that return values; since you're doing imperative work, use if statements that don't need an else.)

Brian
  • 117,631
  • 17
  • 236
  • 300
6

Please don't be afraid to extract functions. This is key to controlling complex logic.

let rec partA () =
  // do some work
  let aValue = makeA ()
  if conditionA 
  then partB aValue 
  else false
and partB aValue =
  // do some work
  let bValue = makeB aValue
  if conditionB 
  then partC bValue
  else false
and partC bValue =
  // do some work
  conditionC 
ChaosPandion
  • 77,506
  • 18
  • 119
  • 157
4

Using the higher-order functions in the Option module can make this flow very cleanly without any mutable state:

let Eval () =
    // do some work
    if not conditionA then None else
        // do some work
        Some state
    |> Option.bind (fun state ->
        if not conditionB then None else
            // do some work
            Some state')
    |> Option.bind (fun state ->
        if not conditionC then None else
            // do some work
            Some true)
    |> defaultArg <| false

Or for further clarity, using named functions rather than lambdas:

let Eval () =
    let a () =
        if not conditionA then None else
            // do some work
            Some state
    let b state =
        if not conditionB then None else
            // do some work
            Some state'
    let c state =
        if not conditionC then None else
            // do some work
            Some true
    // do some work
    a () |> Option.bind b |> Option.bind c |> defaultArg <| false
ildjarn
  • 62,044
  • 9
  • 127
  • 211
  • Rather Haskellish (which means I like it) - this exploits the fact that Option is a monad. It's a shame F# doesn't come with a computation expression for the Option monad really. There are some implementations around on the web, but it'd be great if it was in the core. – Matthew Walton Aug 17 '12 at 08:24
1

You could make your code into a kind of truth table, which depending on your real-world case might make it more explicit:

let condA() = true
let condB() = false
let condC() = true

let doThingA() = Console.WriteLine("Did work A")
let doThingB() = Console.WriteLine("Did work B")
let doThingC() = Console.WriteLine("Did work C")

let Eval() : bool =
    match condA(), condB(), condC() with
    | true,  false, _     -> doThingA();                           false;
    | true,  true,  false -> doThingA(); doThingB();               false;
    | true,  true,  true  -> doThingA(); doThingB(); doThingC();   true;
    | false, _,     _     ->                                       false;
Kit
  • 2,089
  • 1
  • 11
  • 23
  • ...although less efficient as condB() and condC() could get called unnecessarily. – Kit Aug 17 '12 at 07:13
  • To see the power of this approach BTW, try commenting out, say the second match condition. In VS, you get a blue wiggly and a warning that you haven't covered all the cases. – Kit Aug 17 '12 at 08:55
  • 1
    I like this one, pattern matching can be extremely clear. This also has a distinct erlang feel to it :) – 7sharp9 Aug 17 '12 at 09:28
  • If you reversed the order it could look a bit nicer - you would get `|true,true,true -> ... |true,true,_ -> ... |true,_,_ -> ... | _ -> ...` – John Palmer Aug 17 '12 at 10:14
  • 2
    It will not work at all if conditions depend on previous computations, e.g. `let a = f1() in if a=42 then false else let b = f2(a) in if b="foobar" then false else ...` – Be Brave Be Like Ukraine Aug 19 '12 at 14:00
  • @bytebuster Hence 'depending on your real world case...'. – Kit Aug 19 '12 at 19:19