SupportsTrolleyBus API

[kianzarrin]

I don't know if TMPE path manager is going to change its mind about how trolleybus is handled. So I introduced API.

[krzychu124]

Now I'm confused. What is so TM:PE specific there you need an API for it? You have to pass TM:PE lane transition (so you have it) and id of the other lane. That's not what for the API is constructed. You can check all that by yourself, outside of Routing manager since to call that method you already need to have all data, I don't see any code accessing internals that would require the method to be there. Better, it could be even static.

Pushing things to the API just because maybe someone in the future will use it is bad design that will end up with ton of methods that have to be supported later putting a lot of constraints on how things should work, since adding things to API basically tells outside users that this or that thing won't change and can be used so represents a contract - we don't need it yet so it makes no sense to pushing that thing to the API in my opinion.
If we need that then you can ignore that part of the comment.

[kianzarrin]

I can check if lane is trolleybus or not. so some of parts of the code in the API I can do outside of the API too. but the bit that allows relaxed transitions to be used by trolleybuses is the part that is most useful to me. for the API to be complete I should check if lanes are trolleybus because if they are not then they can't be used by trolleybuses. so that is for the sake of completeness.

Anyway that is my thinking if you think the API can be improved (eg I should drop the other lane and it would still be future proof) then please let me know.

[krzychu124]

the bit that allows relaxed transitions to be used by trolleybuses is the part that is most useful to me.

Where is that code though? Like I already said you can comment out your new API method in the interface and make the RoutingManager implementation method static which basically means that the whole method could be created completely outside of RoutingManager, even better, it can be outside of TM:PE assembly too - you just need the API to be able to get LaneTransitionData, so again, my concerns is: why add method to the API if we (TM:PE) don't need it, don't use it and it does not have any special meaning for us with regards to how RoutingManager works.

If just to be sure that when we change something with regards to trolleybuses you could still use it? I really doubt that will work.

What if we come up to completely different idea of checking support, maybe more realistic/dedicated?

You would have to adjust your code anyways,

What if checking the support won't be possible to determine only by lane transition and the other lane id?

We would have to deprecate the API method (which shouldn't be there in the first plane...) so you will need to change your code too.

This change is unnecessarily constraining our code.

Think about the API as a contract that shouldn't change. You can add new things but old things should still work if they are not deprecated completely. In the current state that new method is clearly a helper, not even for TM:PE. I'm not sure if it fits in the API but perfectly fits into helpers assembly thing (has more sense). Some of the things are already in the API but they shouldn't be there.

[kianzarrin]

What if checking the support won't be possible to determine only by lane transition and the other lane id?

the method has access to both source and target lane (target lane from forward transition). how can you not know if Trolleybus can go from source to target lane? the transition might be a bit redundant in this case actually.

maybe I can accept source and target lane ids only in the API.

make the RoutingManager implementation method static

which API class would I put the static method in?