Sometimes is it worth to rethink the problem. Especially when your condition is based on set-members. Using quantor logic often simplifies the condition :
return any(for x in X if x==condition_a) or all(y for y in Y if y==condition_b) and all(x for x in X if x==condition_c)I am not sure if JS has something similar, but this often helps by a lot
Looks like it’s JavaScript, but in Java I would prefer to use the Stream API, something like this:
return availableDrivers.stream() .filter(driver -> calculateDistance(rider, driver) < 5) .filter(driver -> isPreferredVehicle(rider, driver)) .filter(driver -> meetsRiderPreferences(rider, driver)) .findFirst() .orElse(null);Then we have:
private boolean meetsRiderPreferences(Rider rider, Driver driver) { if (driver.rating >= 4.5) { if (rider.preferences.includes('Premium Driver')) { return driver.isPremiumDriver; } else { return true; } } else if (driver.rating >= 4.0) { return true; } else { return false; } }This increases the separation of concern in a neat way, and it becomes more clear what the for loop does at a glance (get the first driver satisfying a set of conditions). The more complicated logic is isolated in meetsRiderPreferences, which now only returns true or false. Reading the method is more about making a mental map of a truth table.
It’s also easy to expand the logic (add more filter conditions, sort the drivers based on rating and distance, break out meetsRiderPreferences into smaller methods, etc.).
Not sure how the equivalent in JavaScript would look like, but this is what I would do in Java.
I try to prefer
.findAny()over.findFirst()because it will perform better in some cases (it will have to resolve whether they are other matches and which one is actually first before they can terminate - more relevant for parallel streams I think. findAny short circuits that) but otherwise I like the first. I’d probably go with some sort of composed predicate for the second, to be able to easily add new criteria. But I could be over engineering.I mostly just posted because I think not enough people are aware of the reasons to use findAny as a default unless findFirst is needed.
For me I have the habit of doing findFirst because determinism is important where I work. But I agree with you if determinism is not of importance.
Using early returns and ternary conditional operator changes
private boolean meetsRiderPreferences(Rider rider, Driver driver) { if (driver.rating >= 4.5) { if (rider.preferences.includes('Premium Driver')) { return driver.isPremiumDriver; } else { return true; } } else if (driver.rating >= 4.0) { return true; } else { return false; } }to
private boolean meetsRiderPreferences(Rider rider, Driver driver) { if (driver.rating < 4.0) return false; if (driver.rating < 4.5) return true; return rider.preferences.includes('Premium Driver') ? driver.isPremiumDriver : true; }
This doesn’t get rid of the if statements. It hides them.
It doesn’t hide. It makes them happen first and, here’s the important bit, closes their scope quickly.
It has conditionals not but actual if statements. Not really different in functionality but a more consistent style.
EDIT: read the article turns out it’s super useful… It gives insight into decision table which is a pattern I did not know about until recently…
Is this really a recurring design pattern for y’all?
I mean, you can just use a switch. anyways I’ll read the article and see ;)
Decision tables are nice. They hide the important part of the logic away out of view of another programmer trying to figure out a bug in the code.
Very helpful! You take longer to find and fix bugs, and potentially miss a few extra ones because of stuff like this. Increased tech debt. Highly recommended! 👍
Also take a look at the Specification Pattern for something similar.
That’s something I would only use if the logic becomes very complex, but it can help break things down nicely in those cases.
I can’t find it right now, but there is some explanation in “Clean Code” why switches shouldn’t be used all over the place.
Google for “replace conditional with polymorphism”.
Just checked and it is in “Clean Code” - Chaper 17; Section G23 “Prefer Polymorphism to if/else or switch/case”.
Thank you!
While I can get behind most of the advice here, I don’t actually like the conditions array. The reason being that each condition function now needs additional conditions to make sure it doesn’t overlap with the other condition functions. This was much more elegantly handled by the
elseclauses, since adding another condition to the array has now become a puzzle to verify the conditions remain non-overlapping.To each their own. Some won’t like the repeating code and some won’t like the distributed logic (i.e. you have to read every if and if-else statement to know when the else takes effect). I think the use of booleans like
isDriverClosemakes the repeated logic less messy and reduces inefficiency (if the compiler didn’t optimize for you).
My issue with this is that it works well with sample code but not as well with real-world situations where maintaining a state is important. What if
rider.preferenceswas expensive to calculate?Note that this code will ignore a rider’s preferences if it finds a lower-rated driver before a higher-rated driver.
With that said, I often work on applications where even small improvements in performance are valuable, and that is far from universal in software development. (Generally developer time is much more expensive than CPU time.) I use C++ so I can read this like pseudocode but I’m not familiar with language features that might address my concerns.
I love how this tries to sell making your code strictly worse as something positive.
Sigh. And it’s still full of ifs.
As a Jr Developer, I found this very helpful. Thanks.
You might want to think about it a bit more before putting it to work. The comment with the streams example is far, far better.
