FreeAndNil commented on code in PR #196: URL: https://github.com/apache/logging-log4net/pull/196#discussion_r1804248055
########## src/log4net.Tests/Appender/AdoNet/Log4NetParameterCollection.cs: ########## @@ -24,45 +24,44 @@ using System.Collections.Generic; using System.Data; -namespace log4net.Tests.Appender.AdoNet +namespace log4net.Tests.Appender.AdoNet; + +public class Log4NetParameterCollection : CollectionBase, IDataParameterCollection { - public class Log4NetParameterCollection : CollectionBase, IDataParameterCollection + private readonly Dictionary<string, int> parameterNameToIndex = new(StringComparer.Ordinal); + + protected override void OnInsertComplete(int index, object? value) { - private readonly Dictionary<string, int> m_parameterNameToIndex = new(StringComparer.Ordinal); + base.OnInsertComplete(index, value); - protected override void OnInsertComplete(int index, object? value) + if (value is IDataParameter param) { - base.OnInsertComplete(index, value); - - if (value is IDataParameter param) - { - m_parameterNameToIndex[param.ParameterName] = index; - } + parameterNameToIndex[param.ParameterName] = index; Review Comment: I understand your reasoning. At my company we had a similar discussion 15 years ago 😅 We decided against _. I think it was mainly because there was only one C developer in the new C# team and the Pascal developers didn't like underscores. The IDE will always show you whether it is a field or a variable. We even have a CA rule which forces you to remove _ or m_ (that's what triggered my m_ changes). But I acknowledge that in pull request it is not that easy (at least without using an IDE). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org