Treat all warnings as errors #49
Conversation
Update the project definition to treat all warnings as errors in the main project. Warnings are still allowed in the test project. Fix errors in the main project. Add HashCode method to the multi value map class. Split multi value map interface into separate file. Reset library version to `0.0.0.0` as it is updated automatically by the build script.
Update private properties to be read-only where possible. Inline variable definitions where possible. Use `[^x]` array indexing instead of `[length - x]` based on C# recommendation.
Add tests for the new GetHashCode method to ensure that two maps that are equal will return the same hashcode. Fix incorrect implementation of the GetHashCode function. The new implementation iterates over the keys, then over all the values and XORs all the hash codes together. Add test to ensure no exceptions are thrown on large maps when calculating the hash code (e.g. Overflow exceptions).
| return true; | ||
| } | ||
|
|
||
| public override int GetHashCode() |
There was a problem hiding this comment.
The issue with this is it becomes quite inefficient. We have a hashcode generation function that's linear in the number of elements in all the value sets. Considering the hash code is used to store elements in things like hash sets, which are supposed to be efficient, this is problematic.
Perhaps a better solution is to remove the equals override and instead add another equality method e.g. mapEquals that's analogoug to the C# function setEquals that's in the set interface.
There was a problem hiding this comment.
I think getting a hashcode is always going to be inefficient. If you are calculating a hash for any data type (string, list, set, etc) you have to iterate over the whole thing at least once to calculate the hash. With equality you can sometimes break early, but not when calculating a hash.
There is another issue with the hashcode though that we should consider: stackoverflow question. The idea is that if you setup your hashcode definition to change when the data in the object changes, and if your object happens to be in a set or dictionary, it can get lost.
ColmBhandal
left a comment
ColmBhandal
left a comment
There was a problem hiding this comment.
Still some issues to go over, particularly with hash code
Rename generic type on the collection extension methods to avoid confusion with the C# definition of "value type". Re-order elements in the multi-value map during testing. Add changelog entry for breaking change to the `TryGetSingleton`.
Description
Update project definition to treat all warnings as errors. If the compiler finds a warning it should not be ignored. Treating it as an error ensures this is the case.
TODO