On Wed, May 29, 2013 at 04:57:54PM +0200, Antonio Quartulli wrote:
+static bool batadv_iv_ogm_metric_is_similar(uint32_t metric,
uint32_t new_metric)+{
- return (metric - new_metric < BATADV_TQ_SIMILARITY_THRESHOLD);
You might want to use abs(metric - new_metric) here, otherwise is_similar(a, b) output might differ from is_similar(b, a).
Mh..imho the name of the function is bad because this has been done on purpose.
agreed, the function name is not really good. You could rename it to something like "is_almost_or_better()" if you keep the current semantics, although this is not an "easy name" either. Or rename it to "similar_or_greater()". Something like that
Although ...
The idea is that we want to see if 'b' is at least as good as 'a', therefore what we want to check if is b is greater than 'a - threshold' only.
Imagine that 'b' is greater than (a + threshold), for me the function has to return true, because the metric b is at least as good as a, but if I introduce the abs() the function would return false.
Example:
a=190 b=240 threshold=20
a - b = -50 < 20 => b is at least as good as a!
using abs:
abs(a - b) = 50 < 20 => NO! b is worse than a....and this is not true.
this situation can happen in the code because usually 'a' will represents some kind of current metric and this is not supposed to be the best ever (maybe we still have to switch to a new best).
... we actually compare to the biggest value (e.g. highest gateway rank, highest bonding candidate), so I wonder if this can really happen. So adding abs() would just make the semantics more clear without breaking the current code when we simply replace. Although we need to check all occurences again, I'm not completely sure that it's always compared to the greatest member.
well the current code uses the semantic that I implemented in the API (IIRC) and this is why I've done so: I wanted to keep the very same behaviour.
I'm also not entirely sure that we always compare to the greatest member.
What you are thinking about is the compare() function taking a threshold as parameter. We can do that with the compare() API if you want, but I think we should keep this "~is_similar()" API in order to avoid behavioural changes in the code.
so I'll go for "bat_metric_is_alike_or_better"...we couldn't find a better name..but if you have one :) let me know
Cheers,