Many years ago, I took part in the development of a taxi-hailing mobile app that is still widely used today. I don’t know what kind of code they’re running now, but in those early days, the driver assignment code –if I remember it correctly– was similar in spirit to the grossly simplified example th...
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 there are other matches and which one is actually first before it 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.
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 else clauses, 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 isDriverClose makes the repeated logic less messy and reduces inefficiency (if the compiler didn't optimize for you).
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(x 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
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! 👍
In case you’re wondering about the down votes, many think Clean Code is not a good book. It got a few good advice, but it also got bad advice disguised as good advice.
I don’t think switch statements should always be avoided. There are cases where polymorphism makes things more difficult to maintain. Saying polymorphism should be used over switch statements is not a good advice.
Here’s an article going into more detail why we should stop recommending Clean Code: https://qntm.org/clean
The fact that the loop is doing "find first driver matching these strange criteria" seems most obviously obscured by the pattern of assigning a value, then killing the loop or not. This strikes me as the part that makes the algorithm tedious to test, since it forces us to use a collection to test the intricacies of the inner conditions.
Once we isolate "find first driver matching condition" from computing the condition for each driver, I consider the rest a question of personal taste. Specification pattern, composition of filters, something like that. Whatever you find easier to follow.