Open jongiddy opened 6 years ago
That's good. It's a nice and simple enhancement of the API and exploits the underlying generic implementation. However, I'm not sure if we should preserve backwards compatibility:
class MatcherKStateGeneric<S extends MatcherSample> extends KState<MatcherCandidate, MatcherTransition, S> {
// implementation of old MatcherKState
}
class MatcherKState extends MatcherKStateGeneric<MatcherSample> {
// constructors
}
and (kind of) overload Matcher#mmatch()
as follows (it's a pity that Java doesn't allow overloading on generic types):
public <S extends MatcherSample> MatcherKStateGeneric<S> mmatchg(List<S> samples,
double minDistance, int minInterval) {
// ...
}
Optionally, also this one:
public <S extends MatcherSample> void mmatch(MatcherKStateGeneric<S> state, List<S> samples,
double minDistance, int minInterval) {
// ...
}
What do you think? I don't want to make the API too complicated.
BTW: I discovered a little design flaw in the implementation of candidate types which have an unnecessary dependency on the sample type as generic argument and which I can resolve with that issue. Only after that it's possible to make k-state generic to MatcherSample
without redefining MatcherCandidate
.
I think the implementation looks fine, but I don't like the names MatcherKStateGeneric
and mmatchg
for the simple reason that the word "generic" is a Java language artifact and has nothing to do with the problem domain.
One possibility is to just use KState
and match
. The matcher KState isn't much different to a markov.KState
apart from some additional output methods. And it's not like anyone is using this code for matching other than map-matching. However, having two methods with such similar names may be error-prone, so maybe matchSamples
or similar.
The method signature that includes passing in the state
looks like it would allow partial processing of a route, and then adding more samples by calling match
on the same state. That sounds like a good idea. And maybe suggests that match
could be called addSamples
.
However, this would also make it possible to modify the KState while using it, so may need some consideration of thread-safety.
In this case, it would also be a good idea to replace List<S> samples
with Iterator<S> samples
to provide more flexibility about input generation. (And document that the iterator must provide samples in time order, so the sort is not required).
It would be useful to have a MatcherKState generic on the sample class;
This would make the code from https://github.com/bmwcarit/barefoot/issues/92#issuecomment-372008797 more type-safe: