You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The equality comparison methods of the Feature class ignore the Id and Properties properties, resulting in e.g. two features with different Id being equal if the geometries are the same.
Since there is a unit test explicitly covering this behavior, I assume it is by design, but I don't understand the rationale behind; for me, it seems very counter-intuitiive and potential source of hard to trackdown bugs. Can somebody give me a clue?
The behavior is especially problematical in conjunction with #56: if one explicitly makes use of the properties of a feature, one most probably will want them to be considered by equality comparison.
On the other hand, it would be very bad to have Feature and Feature<T> behave differently wrt. equality comparison.
Firstly there is no guidance in the RFC for how you should compare two features but it does suggest that different ID's are a different feature which makes sense. I would also agree that two features can share the same geographical space so comparison on geo location alone could be incorrect. If we were to change the existing classes then there would have to be a version bump to highlight the breaking change.
So, should we compare on both id and geometry? Or id and geometry and props? Including the properties in equality comparison might be too expensive for e.g. usage as dictionary keys. But on the other hand one could in such a circumstance use a custom EqualityComparer, so it might be a reasonable default?
I agree that in any case the major version number should be increased, to be consistent with semantic versioning.
Activity
matt-lethargic commentedon Jun 4, 2017
I'll have a closer look shortly, this is code from my predecessor so I'm not 100% sure on the rationale of everything as yet
matt-lethargic commentedon Jun 15, 2017
I've had a look at the code for this and I've refered back to the RFc 7946, but I'm still no wiser. Maybe @jbattermann has more insight into this?
supermihi commentedon Jun 17, 2017
The behavior is especially problematical in conjunction with #56: if one explicitly makes use of the properties of a feature, one most probably will want them to be considered by equality comparison.
On the other hand, it would be very bad to have
FeatureandFeature<T>behave differently wrt. equality comparison.matt-lethargic commentedon Jul 25, 2017
Firstly there is no guidance in the RFC for how you should compare two features but it does suggest that different ID's are a different feature which makes sense. I would also agree that two features can share the same geographical space so comparison on geo location alone could be incorrect. If we were to change the existing classes then there would have to be a version bump to highlight the breaking change.
supermihi commentedon Jul 25, 2017
So, should we compare on both id and geometry? Or id and geometry and props? Including the properties in equality comparison might be too expensive for e.g. usage as dictionary keys. But on the other hand one could in such a circumstance use a custom
EqualityComparer, so it might be a reasonable default?I agree that in any case the major version number should be increased, to be consistent with semantic versioning.
MattFenner commentedon Nov 29, 2018
May want to look at #115 at the same time
xfischer commentedon Feb 26, 2019
OK to close this and refer to #115 ?