• magic_lobster_party@fedia.io
    link
    fedilink
    arrow-up
    29
    ·
    2 months ago

    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.

    • MagicShel
      link
      fedilink
      arrow-up
      10
      ·
      edit-2
      2 months ago

      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.

      • magic_lobster_party@fedia.io
        link
        fedilink
        arrow-up
        8
        ·
        2 months ago

        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.

        • MagicShel
          link
          fedilink
          arrow-up
          3
          ·
          2 months ago

          I would only note that for the vast majority of my experience these streams can only return up to a single match. Determinism isn’t really preserved by findFirst, either, unless the sort order is set up that way.

          Finding the first Jim Jones in a table is no more reliable that finding any Jim Jones. But finding PersonId 13579 is deterministic whether you findFirst or findAny.

          Perhaps you work in a different domain where your experience is different.

    • Kissaki
      link
      fedilink
      English
      arrow-up
      10
      arrow-down
      1
      ·
      edit-2
      2 months ago

      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;
      }
      

      dunno if java has them, but in C# switch expressions could put more of a case focus on the cases

      private boolean meetsRiderPreferences(Rider rider, Driver driver) {
          return driver.rating switch {
              < 4.0 => false,
              < 4.5 => true,
              _      => rider.preferences.includes('Premium Driver') ? driver.isPremiumDriver : true,
          };
      }
      

      or with a body expression

      private boolean meetsRiderPreferences(Rider rider, Driver driver) => driver.rating switch {
          < 4.0 => false,
          < 4.5 => true,
          _      => rider.preferences.includes('Premium Driver') ? driver.isPremiumDriver : true,
      };
      

      The conditional has a true result so it can be converted to a simple bool condition as well.

      private boolean meetsRiderPreferences(Rider rider, Driver driver) => driver.rating switch {
          < 4.0 => false,
          < 4.5 => true,
          _      => !rider.preferences.includes('Premium Driver') || driver.isPremiumDriver,
      };