Let's say I got code like this:
if(conditionA)
do something
if(conditionA && conditionB)
do something more
Obviously, I could nest the ifs (although when there are more complicated conditions this is not readable for me) like below:
if(conditionA)
do something
if(conditionB)
do something more
How can I write similar code in f# idiomatically. I assume if statement isn't considered idiomatic in functional programming (am I wrong?)
I could write two subsequent match expressions or nested match expression to mimic above code but doing so seems pretty ugly to me.
2 Answers 2
There's nothing wrong with using if-then-else
in F#. It's as idiomatic as things can get. Most people do highlight match
expressions when talking about F#, so it may seem that ifs
are discouraged, but that's not really the right way to think of things.
That's true that match
expressions are powerful. It doesn't take much to benefit from using them instead of ifs
when:
- you can use deconstruction in the patterns,
- or you want to match a tuple of values,
- or you have more cases than a binary true/false and want to handle them without building if-else towers.
That said using the construct below should be called out in a code review (and rightly so - this should have been an if):
match cond with
| true -> ...
| false -> ...
It's roughly on the same level of code smelliness as the dreaded construction you might sometimes see in bad c-style lang code:
/// this is bad, don't do this at home (just return cond).
if (cond) {
return true;
} else {
return false;
}
I don't see a problem in your second snippet with nested ifs. That said, there might be better solutions, but it's hard to suggest one when talking about do somethings
and conditionBs
.
-
2Sorry, I can resist to nitpick (and I do get the point of your answer), but please add a footnote that states the cleanest way to write the last snippet is
return cond;
(or simplycond
in an ML-like functional language). Because the rest of your answer makes sense, I'm pretty sure you got confused while translating the first snippet to the second and you probably meant to return something else...Andres F.– Andres F.2015年01月09日 03:36:04 +00:00Commented Jan 9, 2015 at 3:36 -
1@AndresF.: that reaction was spot-on ;) Since that's precisely the (bad) code I wanted to show there. I feel the same when I see the penultimate snippet. Edited for clarity.scrwtp– scrwtp2015年01月09日 07:49:02 +00:00Commented Jan 9, 2015 at 7:49
F# is an expression-oriented language so, as Karl Bielefeldt points out, having if/else
expressions that don't actually return any value is unidiomatic already.
Another aspect of idiomatic functional code is to make it explicit that all possible branches are being addressed. That's one of the reasons why pattern matching with discriminated unions is so popular -- the compiler will warn you if you have forgotten a case.
The if/else
by itself is perfectly acceptable, but it sounds like you are doing something a bit more complicated, when you have complex conditions like this.
If I was looking at this code, I would ask what happens if not conditionA
holds? The code does not make that clear.
If this set of nested conditions occurs frequently, or is a critical piece of code that requires clarity, then I personally would replace the various conditions with a discriminated union to make it obvious to the reader what is going on.
type Keypress =
| AOnly
| AAndB
| NeitherANorB
(Obviously, the names are ugly -- you should rename the cases according to your needs!)
Then create a translator function that turns the input into one of the Keypress
cases:
let inputToKeypress input =
if conditionA then
if conditionB then AAndB else AOnly
else
NeitherANorB
Finally, your main code can pattern match in an obvious way.
let mainCode keypress =
match keypress with
| AOnly -> do something
| AAndB -> do something; do something more;
| NeitherANorB -> () // ignore
An additional advantage of this approach is that if you add or remove cases in your union type, your code will fail to compile. Not so with nested if
expressions.
-
Thank You. What I wanted to achieve is to avoid code duplication. However, in Your solution
do something
appears twice. I'm aware that it might be just a single function call. Look at my second example in question where there's no code duplication at all. I'm still learning so if this obsession of avoiding code duplication has come too far in me, then please let me know.Grzegorz Sławecki– Grzegorz Sławecki2015年01月12日 10:24:39 +00:00Commented Jan 12, 2015 at 10:24 -
1I don't see that avoiding duplication should be a goal in itself, especially if it reduces the clarity of the logic. In the case the logic is: If it's "A only" you do something, if it's "A and B" you do something and also do something more. It's not any less efficient and it's a lot clearer to understand IMO. In my experience, trying to collapse logic together like this can be brittle and causes similar problems to a en.wikipedia.org/wiki/Fragile_base_classGrundoon– Grundoon2015年01月12日 16:19:25 +00:00Commented Jan 12, 2015 at 16:19
let foo = if (blah) then "A" else "B"
is valid. What's less idiomatic is using impure, side-effecting functions in your code. That's going to be impure regardless of whether or not you're using If-statements though (and sometimes it can't be avoided, so don't sweat it).