Added == and != operator overloads to IpAddress

[CoryCharlton]

Description

Added == and != operator overloads

Motivation and Context

There is existing code that does not use the Equals method so this will fix that.

How Has This Been Tested?

On device

Screenshots

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dependencies (update dependencies and changes associated, has no impact on code or features)
• Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
• I have added new tests to cover my changes.

[MrCSharp22]

Great improvement. Looks good to me. Thank you!

[josesimoes]

Interestingly, we are ahead of .NET!! 🤩
Take a look at dotnet/runtime#70978

[josesimoes]

On Sonarcloud bug: I get their point and it makes sense. Looking at the code, there is no major calculation going on with generating the hashcode, so we're probably good with dropping the _hash field and simple compute the hash when needed.
Thoughts @CoryCharlton ?

[CoryCharlton]

On Sonarcloud bug: I get their point and it makes sense. Looking at the code, there is no major calculation going on with generating the hashcode, so we're probably good with dropping the _hash field and simple compute the hash when needed. Thoughts @CoryCharlton ?

I can't see the error now because workflow is re-running but I assume it was something along the lines of "non-readonly field used in GetHashCode" as Resharper gave me the same warning.

I was copying the pattern I saw in the .NET full implementation here. They are doing slightly more work but I think this is also a case of "maximum optimization" as no work is better than some work in a method like GetHashCode where you expect it to be as fast as possible.

That being said I don't have any objections to removing the field and calculating every time so I'll leave it up to you.

[josesimoes]

That's exactly the same complain, yes.
Let's address the warning and recalculate each time. I'm not seeing a big need to cache the hash value instead of recalculating when needed.

[Ellerbach]

Stupid question: who will this play with the larger PR opened by @AdrianSoundy which also touches this?
Shall we wait a bit for the larger one to go and then have this one?
@josesimoes , any thoughts as well? (I know you want to add as well on this one)

[josesimoes]

Stupid question: who will this play with the larger PR opened by @AdrianSoundy which also touches this?

Not stupid at all! It makes sense to have #309 merged first as the merge into this one will be smoother, if not automatic.

[AdrianSoundy]

@CoryCharlton
Should be able to complete this one now as other PR has been merged.

[CoryCharlton]

@AdrianSoundy @josesimoes I merged in the latest changes and think we should be good to go with this. There were several other changes Resharper was recommending to me (switching to var, auto properties, naming [ie: scopeid to scopeId], etc.) but I left them out to not make this change more complicated than it needs to be.

[josesimoes]

@CoryCharlton you did well. PRs need to be atomic. Not good adding other changes, even tiny/random fixes, code style improvements, etc.

[josesimoes]

LGTM. Thanks for this improvement.

[Ellerbach]

@CoryCharlton I think there is an issue.
When doing:

IPAddress ip = default
if( ip == default)
{
  // Should be true but it's not!
}

IPAddress ip = null
if( ip == null)
{
  // Should be true but it's not!
}

[CoryCharlton]

@Ellerbach I'll take a look in a bit, should be a simple fix. In the meantime using the is operator will work as it doesn't use operators defined in the class.

[CoryCharlton]

@josesimoes / @Ellerbach fixed in #311