GHResponse.hasErrors() changed with AlternativeRoutes

Developers
21

I just merged the current master into my codeBase and tried to integrate it.

I would love to see an isValid method, that checks if there is at least one PathWrapper. Currently you have to call getBest and see if there is an Error thrown, if am not mistaken?

An alternative could be that we create a prepared boolean that indicates that a Response is prepared and should contain at least one PathWrapper. If at this point no PathWrapper exist, hasErrors returns true. But this would probably add too much magic?

22

Thanks for trying this out!

What are the cases where there are no errors and no PathWrappers?

23

Indeed the order is to first check the hasErrors and if all are ok then proceed with the response, for path(s) retrieval etc.
That covers all cases, right?

Emux

24

At least it should. If not, that is either a bug or still a conceptually mistake which then needs to be worked on e.g. with the isValid method.

25

Well currently I am doing the following: tmpResonse.hasErrors() || tmpResonse.getAll().isEmpty()

This works for me. I just think it could be nice to wrap that call in a function. But this is not a conceptual issue in my opinion.

Also we should remove the hasRawErrors reference here I guess: https://github.com/graphhopper/graphhopper/blob/master/core/src/main/java/com/graphhopper/GHResponse.java#L101

26

Well currently I am doing the following: tmpResonse.hasErrors() || tmpResonse.getAll().isEmpty()

Why is the hasErrors check not sufficient for your case? Because IF getAll is empty THEN we should have at least one error

ups, done!

27

Sorry, you are right. I was over engineering. Right now I cannot reproduce the Error.

28

This is good. Iā€™m still open for changes if real life tells us we need something different here (for 0.6.0 only until tomorrow ;)).