Fix duplicate DisplayName ArgumentException#1072
Conversation
…s to prevent dictionary key conflicts.
| }; | ||
| cellProperties.Add(cellEntry.DisplayName, property); | ||
|
|
||
| if (cellProperties.ContainsKey(cellEntry.DisplayName)) |
There was a problem hiding this comment.
I think adding the display name of the property to the CellPropertySettings and using a really unique key like the property name would be more preferable here, then just replacing it and logging a warning 🤔 The display name is never a good dictionary key 🙈
Mind, however, you will have to adjust that on the UI side as well then.
There was a problem hiding this comment.
Same for the section in the for loop btw. And if the code is identical for creating the cell properties I think a private method makes sense here 😅
There was a problem hiding this comment.
updated the key handling and property creation
| public Converter(ICustomSerialization serialization, IModuleLogger logger = null) | ||
| { | ||
| Serialization = serialization; | ||
| Logger = logger; |
There was a problem hiding this comment.
It's an internal component. Why this nullability? Complexity could be reduced here when you don't allow null. The logger can also be a private field instead of a public property.
| if (current == null) return null; | ||
|
|
||
| return new ActivityChangedModel | ||
| { | ||
| ResourceId = current.Id, | ||
| }; | ||
| return current == null | ||
| ? null | ||
| : new ActivityChangedModel | ||
| { | ||
| ResourceId = current.Id, | ||
| }; |
There was a problem hiding this comment.
I prefer readability in such cases.
if (current == null)
{
return null;
}
return new ActivityChangedModel
{
ResourceId = current.Id
}| cellProperties.Add(item.Key, item.Value); | ||
| if (cellProperties.ContainsKey(item.Key)) | ||
| { | ||
| Logger?.LogWarning($"Duplicate DisplayName '{item.Key}' in resource '{baseType.Name}'. Overwriting previous value. Ensure unique DisplayNames."); |
There was a problem hiding this comment.
Use Logger.LogWarning("Duplicate DisplayName '{itemKey}' in resource '{baseTypeName}'. Overwriting previous value. Ensure unique DisplayNames.", item.Key, baseType.Name);
… creation via helper
3 participants
This PR adds safe handling for duplicate DisplayName values within ToCellPropertySettings to prevent ArgumentException caused by duplicate dictionary keys.