> The point of passing boolean params instead of named functions is that most of the time there is shared code between the two paths and not literally all the code is enclosed in either the if or the else block. If the author was intending just to restrict to that one specific case where there was no overlap whatsoever, then I guess I would agree but I'm not sure how often that happens.
The cases where there's a lot of commonality are cases for polymorphism instead. If you have something like:
def doSomething(useCache: Boolean) = {
//long and complicated function
if(useCache) cache.lookup(foo) else calculate(foo)
//more long and complicated stuff
}
and passing the FooProvider instead - that way you pass something with a clear semantic meaning rather than a bare Boolean.
> I'd say that at least this trivial example would be best handled by some data-driven approach. Look up salaries in a database instead.
Very much disagree. Every time I've seen a program do logic based on what was in the database it's been very hard to debug or reason about. Anything you can possibly do to make it unit-testable instead of needing to test against a prod database dump is worth it.
> Surely this is easier to misread than the if statement example, which in my mind closely matches how I think.
Disagree. The if looks like control flow when it's not actually control flow. If you're just doing Boolean logic, make it look like Boolean logic.
> But even then, that only works when the actual return type is boolean and there are no state changes made.
Yes, that's exactly the point. It's well worth forcing those things into different functions, so that you can inspect what's going on. Particularly when debugging, that means you can step over the conditional and see what the result was, rather than having to step through each if because you never know when one of the branches might actually do something.
> I don't really see how using an Optional type would remove the if(null) checks. It just makes the meaning explicit, which is good but not remedying the original problem.
Optionals have polymorphic methods so you can express most common use cases directly rather than having to branch, i.e. rather than:
val user = if(null == userId) null else userRepository.lookup(userId)
you can just do
val user = userId flatMap userRepository.lookup
map/flatMap/foreach each have their own specific semantics so it's easy to see what's going on, whereas an "if(null == userId)" could mean anything. (If your logic really doesn't fit into any of the standard use cases then you may need to use an if even with an optional, but such cases will stand out when reading the code - as they should).
The cases where there's a lot of commonality are cases for polymorphism instead. If you have something like:
then it's probably worth breaking that out as and passing the FooProvider instead - that way you pass something with a clear semantic meaning rather than a bare Boolean.> I'd say that at least this trivial example would be best handled by some data-driven approach. Look up salaries in a database instead.
Very much disagree. Every time I've seen a program do logic based on what was in the database it's been very hard to debug or reason about. Anything you can possibly do to make it unit-testable instead of needing to test against a prod database dump is worth it.
> Surely this is easier to misread than the if statement example, which in my mind closely matches how I think.
Disagree. The if looks like control flow when it's not actually control flow. If you're just doing Boolean logic, make it look like Boolean logic.
> But even then, that only works when the actual return type is boolean and there are no state changes made.
Yes, that's exactly the point. It's well worth forcing those things into different functions, so that you can inspect what's going on. Particularly when debugging, that means you can step over the conditional and see what the result was, rather than having to step through each if because you never know when one of the branches might actually do something.
> I don't really see how using an Optional type would remove the if(null) checks. It just makes the meaning explicit, which is good but not remedying the original problem.
Optionals have polymorphic methods so you can express most common use cases directly rather than having to branch, i.e. rather than:
you can just do map/flatMap/foreach each have their own specific semantics so it's easy to see what's going on, whereas an "if(null == userId)" could mean anything. (If your logic really doesn't fit into any of the standard use cases then you may need to use an if even with an optional, but such cases will stand out when reading the code - as they should).