Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In DefaultFareServiceImpl fare calculation logic issue. #319

Open
lbt05 opened this issue Sep 25, 2019 · 0 comments
Open

In DefaultFareServiceImpl fare calculation logic issue. #319

lbt05 opened this issue Sep 25, 2019 · 0 comments

Comments

@lbt05
Copy link

lbt05 commented Sep 25, 2019

Here is the code in org.opentripplanner.routing.impl.DefaultFareServiceImpl.java :

private Fare _getCost(GraphPath path, Set<String> allowedFareIds) {
    List<Ride> rides = createRides(path);
    // If there are no rides, there's no fare.
    if (rides.size() == 0) {
      return null;
    }
    Fare fare = new Fare();
    boolean hasFare = false;
    for (Map.Entry<FareType, Collection<FareRuleSet>> kv : fareRulesPerType.entrySet()) {
      FareType fareType = kv.getKey();
      Collection<FareRuleSet> fareRules;
      if (allowedFareIds != null) {
        fareRules = kv.getValue().stream()
            .filter(f -> allowedFareIds.contains(f.getFareAttribute().getId().toString()))
            .collect(Collectors.toList());
      } else {
        fareRules = kv.getValue();
      }
      // Get the currency from the first fareAttribute, assuming that all tickets use the same
      // currency.
      Currency currency = null;
      if (fareRules.size() > 0) {
        currency =
            Currency.getInstance(fareRules.iterator().next().getFareAttribute().getCurrencyType());
      }
      hasFare = populateFare(fare, currency, fareType, rides, fareRules);
      // log.info("Fares for {} available: {}", fareType, hasFare);
    }
    return hasFare ? fare : null;
  }

inside the for loop, if we have 4 fareTypes need to calculate, let's assume the result as below:
1_fareType : hasFare = true
2_fareType: hasFare = true
3_fareType: hasFare = true
4_fareType: hasFare = false
what will be the result return from this method ? null right ?
I wonder this is written as design or it's a logic error.
if it's a logic error , I advice to update the code to be
hasFare = populateFare(fare, currency, fareType, rides, fareRules) || hasFare;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant