NightOwl888 commented on PR #1056:
URL: https://github.com/apache/lucenenet/pull/1056#issuecomment-2518035078
> This is a draft PR, because I'm currently unsure about whether to override
Equals and GetHashCode (and if so, how exactly to do that, i.e. what to
compare) for QualityQuery. @NightOwl888 - please let me know your thoughts.
>
Well, being that they didn't override the default behavior (which is
reference equality), a direct port would be to override each and then cascade
the call to the base class.
--------------------
But given the fact that the comparer uses `queryID` and it says "ID of this
quality query" for a description, it would probably be best to use that to
compare for equality/hash code. It would keep the equality and compare checks
exactly in sync.
BTW - I noticed that the `QualityQuery.CompareTo()` implementation has an
exception handler that we can remove because we can use `int.TryParse()` and
fallback to string comparison if parsing either side returns `false`.
```c#
public virtual int CompareTo(QualityQuery other)
{
try
{
// compare as ints when ids ints
int n = int.Parse(queryID, CultureInfo.InvariantCulture);
int nOther = int.Parse(other.queryID,
CultureInfo.InvariantCulture);
return n - nOther;
}
catch (Exception e) when (e.IsNumberFormatException())
{
// fall back to string comparison
return queryID.CompareToOrdinal(other.queryID);
}
}
```
The constructor could also be improved by using guard clauses on `queryID`
and `nameValPairs` since there will be NREs if either is passed `null`. If we
do so, we don't need to change `CompareTo()` to account for the fact that
`queryID` may be `null` (which it currently doesn't handle).
`#nullable enable` would catch this sort of thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
