Мелкие вопросы, как всегда, воспринимать как предложения, а не "нужно сделать вот так"
1. Objects.Random нужно или выпилить или хранить в ThreadLocal, он не потокобезопасный
2. Можно в пару предложений — зачем нам нужен exception.GetText()?
Вопрос отпал, rameel ниже ответил.
В exception.GetText() есть мелкий косяк: в конце текста всегда пустая строка.
3. EqualityComparer.GetHashCode():
генерится
{
var num = 321595348; // рандом
num = num * -1521134295 + EqualityComparer<string>.Default.GetHashCode(p.Prop2);
num = num * -1521134295 + EqualityComparer<int>.Default.GetHashCode(p.Field1);
return num;
}
3.1. Надо бы использовать HashCode.Combine() — там используется стандартный алгоритм из Tuple.
3.2. После 3.1. рандомный seed можно убрать, он на распределение особо не влияет
3.3. Было бы круто допилить comparer под реальные сценарии — в 99.99% случаев надо сравнивать по определённым полям. Ну, т.е. нужен builder с синтаксисом аля
var comparerBuilder = new EqualityComparerBuilder<SomeType>
{
{ t => t.PropertyA },
{ t => t.PropertyB.PropertyC },
{ t => t.PropertyC.GetSomeValue() }
};
var comparer = comparerBuilder.ToComparer();
И, возможно, закинуть методы для компарера в Operators<T>, сам код с генерацией — в OperatorsFactory<T> — но это только как всё устаканится.
3.4. С учётом 3.3. А не замахнуться ли по аналогии ещё и на IComparer<T>?
4. AggregateOrDefault — надо бы докинуть перегрузки с TAccumulate, как в стандартном Aggregate().
5. Было бы круто привести код в соответствие с общими соглашениями по форматированию.
Если есть решарпер — он должен подсветить все проблемные места. Если нет — я займусь, но когда время будет — хз.
Уже сделано AVK, спасибо!
В остальном — круто, отдельное спасибо за TypeAccessor
Здравствуйте, Sinix, Вы писали:
S>5. Было бы круто привести код в соответствие с общими соглашениями по форматированию. S>Если есть решарпер — он должен подсветить все проблемные места. Если нет — я займусь, но когда время будет — хз.
Это я уже более менее сделал.
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
Здравствуйте, Sinix, Вы писали:
S>2. Можно в пару предложений — зачем нам нужен exception.GetText()? S> В смысле, чем он настолько крут, что окупает необходимость поддерживать и обычный формат (.ToString()) и новый?
Потому что стандартный Exception.ToString при наличии внутреннего исключения aka InnerException выдает не полный текст исключения. Самый близкий пример AggregateException с несколькими исключениями.
Здравствуйте, Sinix, Вы писали:
S>3.3. Было бы круто допилить comparer под реальные сценарии — в 99.99% случаев надо сравнивать по определённым полям. Ну, т.е. нужен builder с синтаксисом аля S>
S> var comparerBuilder = new EqualityComparerBuilder<SomeType>
S> {
S> { t => t.PropertyA },
S> { t => t.PropertyB.PropertyC },
S> { t => t.PropertyC.GetSomeValue() }
S> };
S> var comparer = comparerBuilder.ToComparer();
S>
Здравствуйте, Sinix, Вы писали:
S>В exception.GetText() есть мелкий косяк: в конце текста всегда пустая строка.
Не всегда. Только после FusionLog была пустая строка, в остальных слочаях только перевод строки. Исправлю.
S>3.1. Надо бы использовать HashCode.Combine() — там используется стандартный алгоритм из Tuple.
То что генерируется сейчас есть стандартный код анонимных типов. Мне в принцмпе по барабану, что скажете то и сгенерируем.
S>3.2. После 3.1. рандомный seed можно убрать, он на распределение особо не влияет
Где именно? В Objects.Random или а GetHashCode компаратора?
S>3.3. Было бы круто допилить comparer под реальные сценарии — в 99.99% случаев надо сравнивать по определённым полям. Ну, т.е. нужен builder с синтаксисом аля
Это легко делается с помощью анонимных типов. Реальный сценарий для публичных полей есть в тестах. Это когда есть список объектов и нужно сделать Distinct по публичным полям. Я это всё, кстати, затеял, т.к. мне в очередной раз понадобилось это на работе и я решил это наконец-то сделать один раз и на века
S>
S> var comparerBuilder = new EqualityComparerBuilder<SomeType>
S> {
S> { t => t.PropertyA },
S> { t => t.PropertyB.PropertyC },
S> { t => t.PropertyC.GetSomeValue() }
S> };
S> var comparer = comparerBuilder.ToComparer();
S>
Можно, конечно. Ещё можно вариант с предикатом, чтобы можно было отобрать поля по условию. Только кешировать здесь не получится. Придётся пользователю самому. Хотя, как я уже сказал, в большинстве реальных сценариев это решается с помощью анонимных типов.
S>И, возможно, закинуть методы для компарера в Operators<T>, сам код с генерацией — в OperatorsFactory<T> — но это только как всё устаканится.
Я исходил из соображений того, что EqualityComparer находится в System.Collections.Generic, поэтому положил всё в Collections.
S>3.4. С учётом 3.3. А не замахнуться ли по аналогии ещё и на IComparer<T>?
Да без проблем. Пожелания отдельным списочком и усё будет у порядке, даже лучше!
S>4. AggregateOrDefault — надо бы докинуть перегрузки с TAccumulate, как в стандартном Aggregate().
Я это даже сделал, потом выкинул. Там получается нужно первый элемент всегда преобразовывать к TAccumulate. Если это устраивает, то добавлю.
S>5. [s]Было бы круто привести код в соответствие с общими соглашениями по форматированию.
Кстати, замена is на as по моему не лучшая идея, т.к. оставляет лишнюю переменную в области видимости где она больше ни разу не используется. А последующей логике ещё и добавляет уровни вложенности. Сравни:
var notFoundException = ex as FileNotFoundException;
if (notFoundException != null)
{
....
}
else
{
var aex = ex as AggregateException;
if (aex?.InnerExceptions != null)
{
....
}
}
[c#]
var notFoundException = ex as FileNotFoundException;
if (ex is FileNotFoundException)
{
....
}
else if (ex is AggregateException)
{
....
}
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, AndrewVK, Вы писали:
AVK>Это тоже доделал. Плюс перегрузки с defaultValue вместо defaultSelector. Много копипасты ввиду все того же перформанса на вызов лямбды
Эх, блин, вот seed мне во всём этом больше всего и не нравится. Его либо взять неоткуда, либо потом func делать с условиями. Нужна лямбда для конвертации первого элемента в TAccumulate. Хотя и это косяк. А самое интересное, что с пустой коллекцией проблемы только для Aggregate<TSource>. Для двух оставшихся перегрузок проблем нет. А мы зачем-то наплодили десяток методов.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, IT, Вы писали:
S>>В exception.GetText() есть мелкий косяк: в конце текста всегда пустая строка. IT>Не всегда. Только после FusionLog была пустая строка, в остальных слочаях только перевод строки. Исправлю.
Криво написал. В общем по-моему и перевода строки тоже не должно быть
S>>3.1. Надо бы использовать HashCode.Combine() — там используется стандартный алгоритм из Tuple. IT>То что генерируется сейчас есть стандартный код анонимных типов. Мне в принцмпе по барабану, что скажете то и сгенерируем.
Ага, мой фейл
Я за тюплы, т.к. они будут чаще использоваться. Особенно после c#7. Но эт только моё мнение, подождём остальных
S>>3.2. После 3.1. рандомный seed можно убрать, он на распределение особо не влияет IT>Где именно? В Objects.Random или а GetHashCode компаратора?
В GetHashCode компаратора.
Если оставлять, то надо его получать один раз и запоминать в static-поле (аля EnumHelper.Holder<T>). А то последовательные вызовы несовместимые компареры будут генерить.
S>>3.3. Было бы круто допилить comparer под реальные сценарии — в 99.99% случаев надо сравнивать по определённым полям. Ну, т.е. нужен builder с синтаксисом аля IT>Это легко делается с помощью анонимных типов.
Вариант конечно, но он не все сценарии покрывает. Например, если надо свой comparer в словарик подсунуть или ещё где переиспользовать.
В общем ок, пусть тогда висит в списке "добавить, как идеи закончатся"
S>>И, возможно, закинуть методы для компарера в Operators<T>, сам код с генерацией — в OperatorsFactory<T> — но это только как всё устаканится. IT>Я исходил из соображений того, что EqualityComparer находится в System.Collections.Generic, поэтому положил всё в Collections.
Ага, с этой точки зрения фигню предложил. Вычёркиваем.
S>>3.4. С учётом 3.3. А не замахнуться ли по аналогии ещё и на IComparer<T>? IT>Да без проблем. Пожелания отдельным списочком и усё будет у порядке, даже лучше!
Ок, тоже к списку добавлю.
S>>4. AggregateOrDefault — надо бы докинуть перегрузки с TAccumulate, как в стандартном Aggregate(). IT>Я это даже сделал, потом выкинул. Там получается нужно первый элемент всегда преобразовывать к TAccumulate. Если это устраивает, то добавлю.
Если ничего не забыл, то TAccumulate seed отдельным аргументом передаётся. AVK эту перегрузку уже добавил.
IT>Кстати, замена is на as по моему не лучшая идея, т.к. оставляет лишнюю переменную в области видимости где она больше ни разу не используется. А последующей логике ещё и добавляет уровни вложенности. Сравни:
А это решарпер насоветовал. Я это место видел, если бы сам правил — 100% оставил бы как есть.
Спорить как правильно не возьмусь, т.к. у меня очень предвзятое мнение по поводу аналайзеров r#, в основном нецензурное
Здравствуйте, Sinix, Вы писали:
S>Криво написал. В общем по-моему и перевода строки тоже не должно быть
S>Я за тюплы, т.к. они будут чаще использоваться. Особенно после c#7. Но эт только моё мнение, подождём остальных
Подождём.
S>>>3.2. После 3.1. рандомный seed можно убрать, он на распределение особо не влияет IT>>Где именно? В Objects.Random или а GetHashCode компаратора? S>В GetHashCode компаратора.
Можно вообще выкинуть. Если специалисты так считают.
S>Если оставлять, то надо его получать один раз и запоминать в static-поле (аля EnumHelper.Holder<T>). А то последовательные вызовы несовместимые компареры будут генерить.
Компаратор генерируется только один раз. А само это значение и так сохранятеся навсегда в Expression.Constant. Хотя, если делать компараторы по заданным полям, то проблема может быть.
S>Вариант конечно, но он не все сценарии покрывает. Например, если надо свой comparer в словарик подсунуть или ещё где переиспользовать. S>В общем ок, пусть тогда висит в списке "добавить, как идеи закончатся"
Да чего там окладывать. Это задача на час. Всего-то чуть расширить существующий код.
S>Если ничего не забыл, то TAccumulate seed отдельным аргументом передаётся. AVK эту перегрузку уже добавил.
Мне вообще вся эта идея с пререгрузками не нравится. Я сделал перегрузку для следующего сценария, если использовать стандартную функцию:
exs.Aggregate((Expression)null, (e1,e2) => e1 == null ? e2 : Expression.AndAlso(e1, e2), e => e ?? Expression.Constant(true));
Здравствуйте, IT, Вы писали:
IT>Кстати, замена is на as по моему не лучшая идея, т.к. оставляет лишнюю переменную в области видимости где она больше ни разу не используется.
Зато убирает двойной тайпкаст. is ведь совсем не бесплатен.
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
Здравствуйте, AndrewVK, Вы писали:
AVK>Зато убирает двойной тайпкаст. is ведь совсем не бесплатен.
В данном случае у нас, во-первых, кеширования, а, во-вторых, там рядом столко кода, что это не будет видно ни в один микроскоп. Впрочем, по большому счёту по барабану.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, AndrewVK, Вы писали:
S>>Угу, спс. Для perftests пока верну таргетинг в 4.6.
AVK>Это последствия борьбы с варнингами при сборке. Там рефы были на msbuild, и студия орала что не может выбрать подходящую версию.
А как воспроизвести? А то у меня классический всёработает, попробую победить
Здравствуйте, Sinix, Вы писали:
AVK>>Это последствия борьбы с варнингами при сборке. Там рефы были на msbuild, и студия орала что не может выбрать подходящую версию. S>А как воспроизвести?
Собрать в студии и посмотреть окошко с варнингами.
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
S>>Криво написал. В общем по-моему и перевода строки тоже не должно быть
IT>
Ну раз no, так no
А не будет лишних переводов строк для logger.WriteLine(ex.GetText())?
Я так понимаю, что это как раз и есть основной сценарий использования.
И на правах "вбросить идею", т.е. я пока сам не уверен что оно надо: не переименвать ли .GetText() в .ToDiagnosticString() или что-то похожее?
Тогда интеллисенс студии будет подсказывать, что помимо обычного .ToString() есть "продвинутый" вариант и новички его будут легче обнаруживать.
IT>Компаратор генерируется только один раз. А само это значение и так сохранятеся навсегда в Expression.Constant. Хотя, если делать компараторы по заданным полям, то проблема может быть.
+1.
IT>Мне вообще вся эта идея с пререгрузками не нравится. Я сделал перегрузку для следующего сценария, если использовать стандартную функцию: IT>Просто и понятно. Есть эффект. А что дадут остальные перегрузки мне непонятно Так что я бы их убрал до возникновения необходимости.
Ну как я понимаю, у нас подход "гуляем на все деньги" — т.е. в библиотеку добавляются мелочи, которые обычный девелопер делать не будет, т.к. банально времени жалко.
В итоге это заметно выделяет CodeJam среди прочих сборников хелперов, по уровню дизайна кода вполне себе на уровне самого фреймворка, что большая редкость.
Здравствуйте, Sinix, Вы писали:
S>И на правах "вбросить идею", т.е. я пока сам не уверен что оно надо: не переименвать ли .GetText() в .ToDiagnosticString() или что-то похожее? S>Тогда интеллисенс студии будет подсказывать, что помимо обычного .ToString() есть "продвинутый" вариант и новички его будут легче обнаруживать.
Принимается.
S>Ну как я понимаю, у нас подход "гуляем на все деньги" — т.е. в библиотеку добавляются мелочи, которые обычный девелопер делать не будет, т.к. банально времени жалко. S>В итоге это заметно выделяет CodeJam среди прочих сборников хелперов, по уровню дизайна кода вполне себе на уровне самого фреймворка, что большая редкость.
Ну не знаю. У меня Aggregate всегда вызывал недоумение своей какой-то бесполезностью и малопонятностью. Дабвить ещё малопонятных перегрузок — только усложнить дело.
ЗЫ.
Добавил ComparerBuilder, который в том числе реализует PublicMemberEqualityComparer. Взгляни, если с первым всё нормально, то второй можно нечаянно пришибить.
Если нам не помогут, то мы тоже никого не пощадим.
Здравствуйте, Sinix, Вы писали: S>И на правах "вбросить идею", т.е. я пока сам не уверен что оно надо: не переименвать ли .GetText() в .ToDiagnosticString() или что-то похожее?
У меня в проекте он называется ExceptionFormatter.FormatException.
Правда он у меня печатает исключение в более наглядной форме, чем родной фреймворка, плюс используются все публичные поля, понимает списки (IEnumerable), словари и объекты. (Для этого используется аналог TypeAccessor, надо бы смерджить вкусности после обсуждения само собой).
Пример печати вложенных исключений
System.InvalidOperationException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName.txt'.
TargetSite: Void Main()
Source: ConsoleApplication1
HResult: -2146233079
Stack Trace:
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 26
Inner Exception:
System.InvalidOperationException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName.txt'.
TargetSite: Void Main()
Source: ConsoleApplication1
HResult: -2146233079
Stack Trace:
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 22
Inner Exception:
System.InvalidOperationException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName.txt'.
TargetSite: Void Main()
Source: ConsoleApplication1
HResult: -2146233079
Stack Trace:
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 18
Inner Exception:
System.IO.DirectoryNotFoundException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName.txt'.
TargetSite: Void WinIOError(Int32, System.String)
Source: mscorlib
HResult: -2147024893
Stack Trace:
at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.File.InternalReadAllBytes(String path, Boolean checkHost)
at System.IO.File.ReadAllBytes(String path)
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 13
Пример печати AggregateException
System.AggregateException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName1.txt'.
TargetSite: Void Main()
Source: ConsoleApplication1
HResult: -2146233088
Stack Trace:
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 26
Inner Exception #0:
System.IO.DirectoryNotFoundException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName1.txt'.
TargetSite: Void WinIOError(Int32, System.String)
Source: mscorlib
HResult: -2147024893
Stack Trace:
at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.File.InternalReadAllBytes(String path, Boolean checkHost)
at System.IO.File.ReadAllBytes(String path)
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 13
Inner Exception #1:
System.IO.DirectoryNotFoundException: Could not find a part of the path 'D:\Test\Path1\Path2\FileName2.txt'.
TargetSite: Void WinIOError(Int32, System.String)
Source: mscorlib
HResult: -2147024893
Stack Trace:
at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
at System.IO.File.InternalReadAllBytes(String path, Boolean checkHost)
at System.IO.File.ReadAllBytes(String path)
at ConsoleApplication1.Program.Main() in E:\Projects\Example\ConsoleApplication1\Program.cs:line 13
S>Тогда интеллисенс студии будет подсказывать, что помимо обычного .ToString() есть "продвинутый" вариант и новички его будут легче обнаруживать.
Здравствуйте, IT, Вы писали:
IT>Ну не знаю. У меня Aggregate всегда вызывал недоумение своей какой-то бесполезностью и малопонятностью. Дабвить ещё малопонятных перегрузок — только усложнить дело.
+1
Но раз уж исправлять, то всё одинаково. Проще сразу сделать нормально, чем потом объяснять, почему тут сделано исключение
IT>Добавил ComparerBuilder, который в том числе реализует PublicMemberEqualityComparer. Взгляни, если с первым всё нормально, то второй можно нечаянно пришибить.
Единственно, я завтра поищу реальные сценарии у нас в проекте. Точно помню, что делал похожую штуку и что у меня потом вылезла какая-то экзотика, под которую код пришлось допиливать. По результатам отпишусь
Здравствуйте, rameel, Вы писали:
R>У меня в проекте он называется ExceptionFormatter.FormatException.
Можно и так.
R>Правда он у меня печатает исключение в более наглядной форме, чем родной фреймворка, плюс используются все публичные поля, понимает списки (IEnumerable), словари и объекты. (Для этого используется аналог TypeAccessor, надо бы смерджить вкусности после обсуждения само собой).
Выглядит куль. Допилишь?
Если нам не помогут, то мы тоже никого не пощадим.