Sort complex forms alphabetically #1389
Conversation
myieye
changed the base branch from
develop
to
feat/1246-allow-reordering-complex-form-components
| public async Task EnsureDefaultVernacularWritingSystemExistsInCrdt() | ||
| { | ||
| // This is optionally called from tests that consume this fixture, so it could get called multiple times in parallel | ||
| await _vernacularSemaphore.WaitAsync(100); |
There was a problem hiding this comment.
I'm not sure this is doing what you expect. If there's already a writing system and no one has entered the semaphore, then it will enter immediately and then return without ever releasing it. The way it's written now all the code should be inside the try. If you don't start the try then the finally will never be called.
| { | ||
| entry.Senses.ApplySortOrder(); | ||
| entry.Components.ApplySortOrder(); | ||
| entry.ComplexForms.Sort((a, b) => ComplexFormsComparison(a, b, sortCompareInfo)); |
There was a problem hiding this comment.
it's a minor thing, but since ApplySortOrder is getting called once for each entry I think it's worth trying to avoid an allocation here (due to capturing sortCompareInfo in the callback). I would change the signature of ApplySortOrder to take an IComparable<ComplexFormComponent>, this basically lets you move the allocation outside of the for each entry loop. Basically you're encapsulating ComplexFormsComparison into a class with a field to store CompareInfo
No description provided.