-
-
Notifications
You must be signed in to change notification settings - Fork 852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't allow duplicate formats in configuration. #684
Conversation
Codecov Report
@@ Coverage Diff @@
## master #684 +/- ##
==========================================
+ Coverage 89.35% 89.35% +<.01%
==========================================
Files 878 878
Lines 39225 39237 +12
Branches 2557 2558 +1
==========================================
+ Hits 35048 35060 +12
Misses 3482 3482
Partials 695 695
Continue to review full report at Codecov.
|
In my project I used ConcurrentDictionary with a byte value which I ignore: https://github.com/tzachshabtay/MonoAGS/blob/master/Source/Engine/AGS.Engine/Misc/Collection/AGSConcurrentHashSet.cs I don't know if it's better than simply locking the hashset. |
@@ -96,5 +92,13 @@ public void AddFormatCallsConfig() | |||
|
|||
provider.Verify(x => x.Configure(config)); | |||
} | |||
|
|||
[Fact] | |||
public void DefaultConfigurationHasCorrectFormatCount() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we also need to test that we don't allow adding duplicates?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll add a simple one.
@tzachshabtay I think I might do that actually; the lock felt ugly. Thanks! Update, the dictionary is breaking two tests as I'd need to implement |
Why do we even try adding duplicates? Maybe we should just remove that part of code. |
We call it when adding both encoders and decoders since they are independent operations |
Don't allow duplicate formats in configuration.
Prerequisites
Description
Fixes #683
ConcurrentBag<T>
allows duplicates which we do not want. I locked aHashSet<T>
since there is noConcurrentSet<T>
type in the .NET Framework.