Надо бы прикрутить к CodeExceptions исключение для выброса, если значение енума невалидно.
Ну и, пользуясь случаем, хочу напомнить Sinix, что он обещал реализацию HasFlag для енумов правильную.

Здравствуйте, AndrewVK, Вы писали:

AVK>Надо бы прикрутить к CodeExceptions исключение для выброса, если значение енума невалидно.
AVK>Ну и, пользуясь случаем, хочу напомнить Sinix, что он обещал реализацию HasFlag для енумов правильную.

Вот не поверишь, в процессе

Раз тему поднял — вопросы по дизайну API:

1. Вот есть у нас метод для проверки enumValue.HasFlags(SomeEnum.SomeValue). И SomeValue = 0 — чего делать?
1.a. Как метод фреймворка — возвращать true. Для HasNoFlag соответственно false возвращать придётся, хотя чисто технически что то, что то — ересь.
1.b. Бросать исключение. Тоже не очень красиво, т.к. никто не будет проверять на "не передаём ли мы 0" и приложение будет радостно крешиться уже в продакшне.


2. Имена методов. Что надо обозвать (в скобках альтернативное название):

2.1. SetFlag     (Include)
2.2. ClearFlag   (Exclude)
2.3. HasFlags    (Includes)
2.4. HasNoFlags  (Excludes)
2.5. HasAnyFlags (IncludesAny)
2.6. ???         (ExcludesAny)

Оба варианта мне не нравятся. HasFlags будут постоянно с встроенным тормозным путать + не подбирается название для 2.6.
Include/Exclude — неочевидно.

Плюс надо учитывать, что для последних 4 будут аналогичные ассерты,

EnumCode.Includes(someValue, nameof(someValue), EnumFlags.SomeFlag);

Т.е. метод должен одинаково нормально смотреться и для проверки на энуме, и для ассерта.

Пока сделаю второй вариант, т.к. он меня раздражает больше — больше шансов, что придумаю что-то нормальное

Здравствуйте, Sinix, Вы писали:

S>1.a. Как метод фреймворка — возвращать true. Для HasNoFlag соответственно false возвращать придётся, хотя чисто технически что то, что то — ересь.
S>1.b. Бросать исключение. Тоже не очень красиво, т.к. никто не будет проверять на "не передаём ли мы 0" и приложение будет радостно крешиться уже в продакшне.

Думаю, консистентность с фреймворком в данном случае важнее.

S>2. Имена методов. Что надо обозвать (в скобках альтернативное название):
S>

S>2.1. SetFlag     (Include)
S>2.2. ClearFlag   (Exclude)
S>2.3. HasFlags    (Includes)
S>2.4. HasNoFlags  (Excludes)
S>2.5. HasAnyFlags (IncludesAny)
S>2.6. ???         (ExcludesAny)
S>

S>Оба варианта мне не нравятся. HasFlags будут постоянно с встроенным тормозным путать + не подбирается название для 2.6.

Для 2.5 и 2.6 можно использовать те же названия, что и для 2.3 и 2.4 — параметров то у них меньше будет. Но по смыслу оно все не очень. Тем более нужно то не только для флагов, но и для обычных енумов.

Здравствуйте, AndrewVK, Вы писали:

AVK>Думаю, консистентность с фреймворком в данном случае важнее.
Ок, так и делаю.

AVK>Для 2.5 и 2.6 можно использовать те же названия, что и для 2.3 и 2.4 — параметров то у них меньше будет. Но по смыслу оно все не очень. Тем более нужно то не только для флагов, но и для обычных енумов.

В принципе согласен. Чтоб не обсуждать впустую скинул тесты + примитивную реализацию, всё лежит в EnumTests.

UPD Мои глаза Мало того что накосячил с ExcludesAny, так ещё и тестами покрыл. Правлю.
UPD2 Нда, теперь точно соласен. Получаются два основных метода, Includes + IncludesAny, парные получаются простым логическим отрицанием. Тесты исправил.

Как их обзывать будем?

	Для истории, неактуально
	Затык с обработкой 0. Ассерты с ними вынес в Test00ToDiscuss, вот с ними надо определиться. Если знаешь, как сделать лучше — можно не обсуждать, просто тесты поправить После того, как определимся — можно будет и имена подобрать, и нормальную реализацию сделать. О, а заведу-ка я в профильном разделе тему. Пусть народ поучаствует.

P.S.
Заодно добавил в Operators<T> поддержку операций для nullable enums (по аналогии с костылями, что компилятор добавляет в expression trees), тестами покрыл.
В релиз переносить необязательно, не так оно важно.

P.P.S.
Чтоб не забыть, вопрос по Operators.AreEqual/AreNotEqual — имена устраивают?
Без префикса Are, как в System.Linq.Expressions.Expression, слишком легко спутать c стандартным Equals.

Здравствуйте, Sinix, Вы писали:

S>Заодно добавил в Operators<T> поддержку операций для nullable enums (по аналогии с костылями, что компилятор добавляет в expression trees), тестами покрыл.
S>В релиз переносить необязательно, не так оно важно.

И не собирался. У меня другой вопрос — а нафига надо было заменять параметры конструктора OpInfo на отдельные методы со свитчами? Какая то экономия на кошках с разнесением единой информации по закоулкам.

S>P.P.S.
S>Чтоб не забыть, вопрос по Operators.AreEqual/AreNotEqual — имена устраивают?

Да.

S>Без префикса Are, как в System.Linq.Expressions.Expression, слишком легко спутать c стандартным Equals.

Согласен.

Здравствуйте, AndrewVK, Вы писали:

AVK>И не собирался. У меня другой вопрос — а нафига надо было заменять параметры конструктора OpInfo на отдельные методы со свитчами? Какая то экономия на кошках с разнесением единой информации по закоулкам.

А, так проще видеть, какие операторы в каждом из кейсов участвуют. Без этого тяжело отслеживать, что добавил, что забыл.

Ну и если новые кейсы добавятся, то проще ещё один свитч написать, чем закидывать ещё один столбец bool-параметров во все места, где используется конструктор.

S>>Чтоб не забыть, вопрос по Operators.AreEqual/AreNotEqual — имена устраивают?
AVK>Да.
Ок.

Вопрос по методам для enum-ов ещё в силе. Как их обзывать, есть идеи?

Здравствуйте, Sinix, Вы писали:

S>А, так проще видеть, какие операторы в каждом из кейсов участвуют. Без этого тяжело отслеживать, что добавил, что забыл.

Спорно. Лично мне проще, когда все в одном месте, а не раскидано по куче методов.

S>Ну и если новые кейсы добавятся, то проще ещё один свитч написать, чем закидывать ещё один столбец bool-параметров во все места, где используется конструктор.

Во все места не надо, на то есть optional параметры и просто writeable свойства со значениями по умолчанию.

S>Вопрос по методам для enum-ов ещё в силе. Как их обзывать, есть идеи?

Мне все таки более правильным кажется IsFlagSet/IsFlagMatch

Здравствуйте, AndrewVK, Вы писали:

AVK>Спорно. Лично мне проще, когда все в одном месте, а не раскидано по куче методов.
ОК, если не нравится — поправь как удобно, ок?

Я сделал так, как сделал после трёх подряд ошибок "забыл оператор/добавил не то".
Плюс заметил, что при старом подходе легко добавить один и тот же оператор дважды, но с разными флагами.
Эт конечно максимализм, но API, которое провоцирует делать ошибки, жить не должно


AVK>Во все места не надо, на то есть optional параметры и просто writeable свойства со значениями по умолчанию.
Ну сам представь насколько удобно, скажем, найти все operator для enum-ов (туда ещё == и != добавил, скинул).


S>>Вопрос по методам для enum-ов ещё в силе. Как их обзывать, есть идеи?
AVK>Мне все таки более правильным кажется IsFlagSet/IsFlagMatch

все будут использовать IsFlagSet, как не вызывающий вопросов. А он у тебя делает не совсем то, что ожидается

if(4.IsFlagSet(4|8)) // True, huh????
{
}

Ну и нам нужны имена для всех 4 методов. Они в ассертах точно будут использоваться.

Здравствуйте, Sinix, Вы писали:

S>все будут использовать IsFlagSet, как не вызывающий вопросов. А он у тебя делает не совсем то, что ожидается
S>[cs]
S>if(4.IsFlagSet(4|8)) // True, huh????

Почему true то? (value & flag) == flag вернет false.

S>Ну и нам нужны имена для всех 4 методов. Они в ассертах точно будут использоваться.

Ну так вроде все очевидно: IsFlagSet/IsFlagNotSet/IsFlagMatch/IsFlagNotMatch

Здравствуйте, AndrewVK, Вы писали:

AVK>Почему true то? (value & flag) == flag вернет false.

Понял свой косяк. В идеале надо вариант с Match переобозвать. А то вариант он как более строгий воспринимается, типа есть строгое соответствие.

AVK>Ну так вроде все очевидно: IsFlagSet/IsFlagNotSet/IsFlagMatch/IsFlagNotMatch
Поскольку идеального решения пока нет, остановимся на этом. Уж точно лучше моего начального

Здравствуйте, AndrewVK, Вы писали:

AVK>Надо бы прикрутить к CodeExceptions исключение для выброса, если значение енума невалидно.
AVK>Ну и, пользуясь случаем, хочу напомнить Sinix, что он обещал реализацию HasFlag для енумов правильную.

Скинул инфраструктурную часть (Holder<T>) + методы IsDefined/TryParse. Зачем нужны — см аннотации в perftests (вот не зря я на них время тратил ).
Например, IsDefined в 28 раз быстрее. Для "досталось задаром" — неплохо.

Дальнейшие планы:
* собственно флаги (в процессе).
* extension methods для nullable enums
* Рефакторинг — EnumExtensions надо или переименовать, или вытащить IsDefined/TryParse в отдельный класс. + убрать дубликаты из CodeJam.Reflection.EnumHelper.
* Сode assertions для энум-ов.

Здравствуйте, AndrewVK, Вы писали:

AVK>Надо бы прикрутить к CodeExceptions исключение для выброса, если значение енума невалидно.
AVK>Ну и, пользуясь случаем, хочу напомнить Sinix, что он обещал реализацию HasFlag для енумов правильную.

EnumExtensions done. Предложения по дизайну приветствуются.

Что как минимум надо сделать:

1. Мне не нравится проверка на IsDefined —

bool defined = EnumExtensions.IsDefined(SomeEnum.SomeValue)

Или переименовать EnumExtensions, или вытащить IsDefined в отдельный тип, или делать IsDefined расширением. Что лучше?

2. Перегрузки для Nullable<TEnum>. На мой взгляд не нужны, т.к. в шарпе уже есть удобный синтаксис для null propagation.

3. Расширения компилируются для любых чисел, в т.ч. double,

var a = 2.SetFlag(3);
var b = 3.0.SetFlag(5.0);

но бросают ArgumentException, если тип — не Enum.
Есть идеи, как сделать поведение лучше — велкам! Как минимум, я собираюсь корректно работать с целочисленными типами, но сначала хочу выслушать ещё предложения.


Как определимся — сделаю ассерты для энумов.

Здравствуйте, Sinix, Вы писали:

S>Или переименовать EnumExtensions, или вытащить IsDefined в отдельный тип, или делать IsDefined расширением. Что лучше?

Лучше, наверное, переименовать. Расширение непонятно к чему делать.

S>2. Перегрузки для Nullable<TEnum>. На мой взгляд не нужны, т.к. в шарпе уже есть удобный синтаксис для null propagation.

В 6. В более ранних — нет. Вообще некоторое количество таких методов, малополезных при наличии элвиса уже есть.

S>Есть идеи, как сделать поведение лучше — велкам! Как минимум, я собираюсь корректно работать с целочисленными типами, но сначала хочу выслушать ещё предложения.

Не знаю. Сделав енум недотипом, МС, конечно, перформанс улучшил, но в плане дизайна свинью изрядную подложил. Так что, что так, что эдак — все равно фигово.

Здравствуйте, AndrewVK, Вы писали:

AVK>Лучше, наверное, переименовать. Расширение непонятно к чему делать.
Ок. EnumHelper пойдёт?


S>>2. Перегрузки для Nullable<TEnum>. На мой взгляд не нужны, т.к. в шарпе уже есть удобный синтаксис для null propagation.
AVK>В 6. В более ранних — нет. Вообще некоторое количество таких методов, малополезных при наличии элвиса уже есть.

Тогда я их копипастой сделаю, через t4 тяжеловато. Для SetFlag/ClearFlag поведение будет аналогично остальным lifted ops: любой операнд null — результат тоже null.

что делать с проверками вида x.IsFlagSet(nullValue) и nullValue.IsFlagSet(x) ?
Тоже по аналогии с lifted comparison ops: любой операнд null — результат false?



S>>Есть идеи, как сделать поведение лучше — велкам! Как минимум, я собираюсь корректно работать с целочисленными типами, но сначала хочу выслушать ещё предложения.
AVK>Не знаю. Сделав енум недотипом, МС, конечно, перформанс улучшил, но в плане дизайна свинью изрядную подложил. Так что, что так, что эдак — все равно фигово.

Ок, пока не трогаю, жду других предложений.

Здравствуйте, Sinix, Вы писали:

AVK>>Лучше, наверное, переименовать. Расширение непонятно к чему делать.
S>Ок. EnumHelper пойдёт?

Подойдет. Но нужно будет смержить с существующим.

S>Тоже по аналогии с lifted comparison ops: любой операнд null — результат false?

Не знаю. Сложно сказать в отрыве от реального применения.

Здравствуйте, AndrewVK, Вы писали:

AVK>Подойдет. Но нужно будет смержить с существующим.
Сделаю.


AVK>Не знаю. Сложно сказать в отрыве от реального применения.
Да логично всё вроде:

enumValue.HasFlag(0) - true
enumValue.IsFlagSet(0) - true
enumValue.IsFlagSet(null) - false

nullValue?.HasFlag(0) - null
nullValue.IsFlagSet(0) - false

Проверки с Not, например, IsFlagNotSet, будут всегда инвертировать результат парной проверки, иначе запутаемся.

IsDefined и TryParse, понятно, будут false возвращать для null-ов

Здравствуйте, Sinix, Вы писали:

Ты поглядывай на серверные сборки — за последнее время ты регулярно их ломаешь. Опять под 4.0 не собирается.

  EnumHelper.cs(79,10): error CS1729: 'DictionaryWithReadOnly<string, TEnum>' does not contain a constructor that takes 1 arguments [S:\Work\CodeJam\Main\src\CodeJam.Main.csproj]

UPD: я поправил, но на будущее следи. Лучше всего подпишись на нотификации, чтобы тебе письмо при смене статуса сборок приходило.

Здравствуйте, AndrewVK, Вы писали:

AVK>UPD: я поправил, но на будущее следи. Лучше всего подпишись на нотификации, чтобы тебе письмо при смене статуса сборок приходило.


Буду перепроверять.

Да, а как в appveyor на чужой проект подписаться? там же емнип сначала team в составе проекта заполнить.

Здравствуйте, Sinix, Вы писали:

S>Да, а как в appveyor на чужой проект подписаться? там же емнип сначала team в составе проекта заполнить.

По идее если залогинится через github, то доступ должен появиться. Гитхабный тим я прописал.

Здравствуйте, AndrewVK, Вы писали:

S>>Да, а как в appveyor на чужой проект подписаться? там же емнип сначала team в составе проекта заполнить.

AVK>По идее если залогинится через github, то доступ должен появиться. Гитхабный тим я прописал.

Не работает.
Настроил копию чисто для проверки — https://ci.appveyor.com/project/ig-sinicyn/codejam

зайди проверь плиз, у тебя по этой ссылке ней правее "Latest build" settings доступны?

Если да — надо в твоём appveyor вот тут дать github team права администраторов. Или в users добавить и выставить в settings — permissions все галочки кроме delete.

Как проверишь — напиши, чтоб я свой appveyor-билд сразу грохнул.