EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 09:06
Оценка:
Вести с полей: выбранные вот тут
Автор: Sinix
Дата: 23.04.16
имена методов EnumHelper.IsFlagSet / EnumHelper.IsFlagMatch внезапно оказались полным отстоем — их постоянно путали, вплоть до ловли ошибок уже на стадии QA.

В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.
Понятно, что изменение ломающее,
но остальные варианты: оставить как есть | добавить правильные имена, старые пометить obsolete — ещё хуже.

А, да, ассерты для энумов будут, скоро. Сразу с новыми именами, если что — переименуем в старые.
Re: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 17.09.16 11:54
Оценка: 46 (1) +1
Здравствуйте, Sinix, Вы писали:

S>Вести с полей: выбранные вот тут
Автор: Sinix
Дата: 23.04.16
имена методов EnumHelper.IsFlagSet / EnumHelper.IsFlagMatch внезапно оказались полным отстоем — их постоянно путали, вплоть до ловли ошибок уже на стадии QA.


S>В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.


Тоже не кристально понятно.
Есть мнение, что флаговые енамы можно представлять как множества и использовать термины из теории множеств. При этом не создавать специальные методы для одного/нескольких флагов.
IsFlagSet() выражается через Contains()[Includes()]
IsAnyFlagSet() через Intersects() (подразумевается непустое пересечение двух множеств).

***Unset() идут лесом
Re: EnumHelper: request for breaking changes
От: AndrewVK Россия http://blogs.rsdn.org/avk
Дата: 17.09.16 12:07
Оценка: +1
Здравствуйте, Sinix, Вы писали:

S>В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.


Не против.

S>Понятно, что изменение ломающее,


В 1.2 уже одно ломающее есть — изменение дефолтного значения параметра в LazyDictionary. Может тогда стоит сделать 2.0?
... << RSDN@Home 1.0.0 alpha 5 rev. 0 on Windows 8 6.2.9200.0>>
AVK Blog
Re[2]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 12:19
Оценка:
Здравствуйте, samius, Вы писали:

S>>В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.

S>Тоже не кристально понятно.
Не спорю, но код, который позволяет спутать "все флаги выставлены" и "любой флаг выставлен" — точно не вершина дизайна

Как понял, названия подбирали по реальному коду, т.е. хоть кого-то этот вариант устроил



S>Есть мнение, что флаговые енамы можно представлять как множества и использовать термины из теории множеств. При этом не создавать специальные методы для одного/нескольких флагов.


+1.
В оригинальном дизайне оно так и было, но это было лет так 7 назад. В чём косяк:
у нас кроме методов для проверки есть ещё методы для манипуляций с флагами:
options = options.SetFlag(Options.DoThis);
options = options.ClearFlag(Options.DoThat);


как их обзывать-то?
options = options.With(Options.DoThis).Without(Options.DoThat);
// или
options = options.Include(Options.DoThis).Exclude(Options.DoThat);

обои хуже, кмк.


S>***Unset() идут лесом

Нужны для ассертов, аля
EnumCode.FlagUnset(options, nameof(options), Options.DoThis | Options.DoThat); // asserts that both flags are unset
EnumCode.AnyFlagUnset(options, nameof(options), Options.DoThis | Options.DoThat); // asserts that any flag cleared.

Соответствующие методы в самом EnumHelper — чисто для компании.
Re[2]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 12:24
Оценка:
Здравствуйте, AndrewVK, Вы писали:

S>>В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.

AVK>Не против.
Ну вот Samius ещё идеи подкинул, сейчас обмозгуем и сделаем.


S>>Понятно, что изменение ломающее,

AVK>В 1.2 уже одно ломающее есть — изменение дефолтного значения параметра в LazyDictionary. Может тогда стоит сделать 2.0?
Я бы не парился. Если строго следовать semver, то у нас сейчас 0.2, а в 0.x допускаются ломающие изменения:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

Понятно, что поздняк метаться, плюс мы в хорошей компании — .Net Core вообще в релиз пошёл с уже выброшенным json project
Re[3]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 17.09.16 13:03
Оценка:
Здравствуйте, Sinix, Вы писали:

S>>Тоже не кристально понятно.

S>Не спорю, но код, который позволяет спутать "все флаги выставлены" и "любой флаг выставлен" — точно не вершина дизайна
Согласен

S>Как понял, названия подбирали по реальному коду, т.е. хоть кого-то этот вариант устроил

Возможно, найдется вариант, который устроит более чем хоть кого-то.

S>>Есть мнение, что флаговые енамы можно представлять как множества и использовать термины из теории множеств. При этом не создавать специальные методы для одного/нескольких флагов.


S>+1.

S>В оригинальном дизайне оно так и было, но это было лет так 7 назад. В чём косяк:
S>у нас кроме методов для проверки есть ещё методы для манипуляций с флагами:
S>
S>options = options.SetFlag(Options.DoThis);
S>options = options.ClearFlag(Options.DoThat);
S>

Эти названия не путают, но не сочетаются с операциями над множествами, так что, или или.

S>как их обзывать-то?

S>
S>options = options.With(Options.DoThis).Without(Options.DoThat);
S>// или
S>options = options.Include(Options.DoThis).Exclude(Options.DoThat);
S>

S>обои хуже, кмк.
аналогом будут Union/Complement(Difference)

S>>***Unset() идут лесом

S>Нужны для ассертов, аля
S>
S>EnumCode.FlagUnset(options, nameof(options), Options.DoThis | Options.DoThat); // asserts that both flags are unset
S>EnumCode.AnyFlagUnset(options, nameof(options), Options.DoThis | Options.DoThat); // asserts that any flag cleared.
S>

S>Соответствующие методы в самом EnumHelper — чисто для компании.
NotIntersects[With]()
Причем, что для одного флага, что для нескольких.

И положить их в EnumSet хелпер...
Re[4]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 13:49
Оценка:
Здравствуйте, samius, Вы писали:

S>>Как понял, названия подбирали по реальному коду, т.е. хоть кого-то этот вариант устроил

S>Возможно, найдется вариант, который устроит более чем хоть кого-то.

Невопрос, сейчас организуем
Табличка с вариантами открыта на запись, кто хочет — предлагайте!

* Если я напутал с тем, что ты предлагаешь — смело правь
Re[5]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 17.09.16 14:42
Оценка: 23 (1)
Здравствуйте, Sinix, Вы писали:

S>Табличка с вариантами открыта на запись, кто хочет — предлагайте!


S>* Если я напутал с тем, что ты предлагаешь — смело правь


поправил.
По-моему "if any flag unset" и "if all flags are cleared" это одно и то же. Или пересечение пусто.
+ нельзя сказать множеству включи или исключи. Множества не выполняют императивные инструкции. А операции над множествами — они просто определены и все. Как в функциональщине. Никакого императива.

Еще один момент. В моем коде есть операция SetFlag, но она имеет следующую нагрузку
fileAccess = fileAccess.SetFlag(FileAccess.Write, value);
Очень полезно, когда не знаем, надо ли установить флаг, или снять.
Re[6]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 17:28
Оценка:
Здравствуйте, samius, Вы писали:

S>По-моему "if any flag unset" и "if all flags are cleared" это одно и то же. Или пересечение пусто.

Неа.
(A|B).IsAnyFlagUnset(B|C|D) == true // C or D are unset
(A|B).IsFlagUnset(B|C|D) == false   // B is set


В общем, IsXxxUnset-проверки — это логическое отрицание соответствующих IsXxxSet-методов



S>Еще один момент. В моем коде есть операция SetFlag, но она имеет следующую нагрузку

S>fileAccess = fileAccess.SetFlag(FileAccess.Write, value);
S>Очень полезно, когда не знаем, надо ли установить флаг, или снять.

Вот тут не понял, но интересно. Можно подробее?
Re[7]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 17.09.16 18:57
Оценка:
Здравствуйте, Sinix, Вы писали:

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


S>>По-моему "if any flag unset" и "if all flags are cleared" это одно и то же. Или пересечение пусто.

S>Неа.
Т.е. я как раз продемонстрировал невтыкаемость в данную терминологию
S>
S>(A|B).IsAnyFlagUnset(B|C|D) == true // C or D are unset
S>

Я бы записал это как
(B|C|D).Diff(A|B).Any()

S>
S>(A|B).IsFlagUnset(B|C|D) == false   // B is set
S>

Опять не понял. C и D ведь unset! Можно выразить это в операциях на множествах?


S>В общем, IsXxxUnset-проверки — это логическое отрицание соответствующих IsXxxSet-методов

Окей, значит я и их тоже не понял.


S>>Еще один момент. В моем коде есть операция SetFlag, но она имеет следующую нагрузку

S>>fileAccess = fileAccess.SetFlag(FileAccess.Write, value);
S>>Очень полезно, когда не знаем, надо ли установить флаг, или снять.

S>Вот тут не понял, но интересно. Можно подробее?

value указывает, включить или выключить соответствующий флаг. Например, приходит как значение некого булевого свойства, которое хранится в Enum-е.
FileAccess SetFlag(this FileAccess left, FileAccess right, bool value)
            => value ? left | right : left & ~right;
Re[8]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 17.09.16 19:07
Оценка:
Здравствуйте, samius, Вы писали:

S>Я бы записал это как

Речь про типовую операцию, требовать 2 метода на ровном месте низзя


S>>(A|B).IsFlagUnset(B|C|D) == false // B is set

S>Опять не понял. C и D ведь unset! Можно выразить это в операциях на множествах?
Проверяет, что пересечение — пустое множество, можно переобозвать как (A|B).AllFlagsUnset(B|C|D) == false


S>>Вот тут не понял, но интересно. Можно подробее?

S>value указывает, включить или выключить соответствующий флаг. Например, приходит как значение некого булевого свойства, которое хранится в Enum-е.
А, можно, но неочевидно как-то вместе с простым SetFlag() выглядит.
Добавил "conditional set flag" в сценарии, буду думать.
Re[9]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 17.09.16 19:21
Оценка:
Здравствуйте, Sinix, Вы писали:

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


S>Речь про типовую операцию, требовать 2 метода на ровном месте низзя

на ровном месте низзя — а на множествах зя. Как вариант — IsDiffNotEmpty()


S>>>(A|B).IsFlagUnset(B|C|D) == false // B is set

S>>Опять не понял. C и D ведь unset! Можно выразить это в операциях на множествах?
S>Проверяет, что пересечение — пустое множество, можно переобозвать как (A|B).AllFlagsUnset(B|C|D) == false
Вот как раз проверка непустоты пересечения — более понятна, чем вот это с ансетами.

S>>value указывает, включить или выключить соответствующий флаг. Например, приходит как значение некого булевого свойства, которое хранится в Enum-е.

S>А, можно, но неочевидно как-то вместе с простым SetFlag() выглядит.
S>Добавил "conditional set flag" в сценарии, буду думать.
А простой SetFlug выглядит просто как затычка для симметрии, что бы был. Уж лучше пользоваться OR-ом.
Re[2]: EnumHelper: request for breaking changes
От: Lexey Россия  
Дата: 20.09.16 14:26
Оценка:
Здравствуйте, AndrewVK, Вы писали:

S>>Понятно, что изменение ломающее,


AVK>В 1.2 уже одно ломающее есть — изменение дефолтного значения параметра в LazyDictionary. Может тогда стоит сделать 2.0?


А в 1.1 Option ломали, так что, начало уже было положено.
"Будь достоин победы" (c) 8th Wizard's rule.
Re[2]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 21.09.16 08:04
Оценка:
Здравствуйте, AndrewVK, Вы писали:

S>>В итоге народ переобозвал методы в IsFlagSet() / IsAnyFlagSet() / IsFlagUnset() / IsAnyFlagUnset() и так пользуется. Собственно предложение: нам сделать так же.

AVK>Не против.

Скинул, если есть предложения, как сделать лучше — вэлкам!

Ассерты (EnumCode) в комплекте. Я их ещё подополняю, дальше будем смотреть — переносить их в Code или пусть отдельно живут.
Лично я за то, чтобы их отдельно держать, но если большинство против — спорить не буду
Re[10]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 21.09.16 08:08
Оценка:
Здравствуйте, samius, Вы писали:

S>>Речь про типовую операцию, требовать 2 метода на ровном месте низзя

S>на ровном месте низзя — а на множествах зя. Как вариант — IsDiffNotEmpty()

В общем как ни крутил, ничего лучше IsAnyFlagSet не придумал, скинул так.

Поспрашивал, твой вариант части народа понравился, остальным не понравился Intersects — типа работа у нас с enum-ами, делаем для типовых программистов, требовать от всех уметь в множества — издевательство.

S>>>value указывает, включить или выключить соответствующий флаг. Например, приходит как значение некого булевого свойства, которое хранится в Enum-е.

Добавил, спасиб за идею!
Re[11]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 21.09.16 09:11
Оценка:
Здравствуйте, Sinix, Вы писали:

S>Поспрашивал, твой вариант части народа понравился, остальным не понравился Intersects — типа работа у нас с enum-ами, делаем для типовых программистов, требовать от всех уметь в множества — издевательство.


Тут мое мнение по части типового программиста расходится с остальнымм народом (кому не понравился Intersects). Насколько я себе представляю, знакомство с теорией множеств входит в стандартный курс вышки, который читают даже студентам нетехнических специальностей. Если типовой программист не знает что такое intersects, то кто же будет писать реализации компареров для объектов, которые требуют немного более глубокого понимания множеств (на уровне отношений порядка)? Наверное, типовой тимлид.

S>Добавил, спасиб за идею!

Не за что.
Re[12]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 21.09.16 09:54
Оценка:
Здравствуйте, samius, Вы писали:


S>>Поспрашивал, твой вариант части народа понравился, остальным не понравился Intersects — типа работа у нас с enum-ами, делаем для типовых программистов, требовать от всех уметь в множества — издевательство.

S>Тут мое мнение по части типового программиста расходится с остальнымм народом (кому не понравился Intersects).

Моё, если честно, тоже. Но вот это правило

DO understand and explicitly design for a broad range of developers with different programming styles, requirements, and skill levels.

с комментариями от людей, уже наступавших на эти грабли

CHRIS SELLS As a test of whether an API is "simple," I like to submit it to a test I call "client-first programming." If you say what your library does and ask a developer to write a program against what he or she expects such a library to look like (without actually looking at your library), does the developer come up with something substantially similar to what you've produced? Do this with a few developers. If the majority of them write similar things without matching what you've produced, they're right, you're wrong, and your library should be updated appropriately.

I find this technique so useful that I often design library APIs by writing the client code I wish I could write and then just implement the library to match that. Ofcourse, you have to balance simplicity with the intrinsic complexity of the functionality you're trying to provide, but that's what your computer science degree is for!


KRZYSZTOF CWALINA It is easy to design for users who are like you, and very difficult to design for somebody urdike you. There are too many APk that are designed by domain experts and, frankly, they are only good for domain experts. The problem is that most developers are not, will never be, and do not need to be experts in aU technologies used in modem applications.


KRZYSZTOF CWALINA We did not test the usability of the types in the System.IO namespace before shipping version 1.0 of the Framework. Soon after shipping, we received negative customer feedback about System.IO usability. We were quite surprised, and we decided to conduct usability studies with eight average VB developers. Eight out of eight failed to read text from a file in the 30 minutes we allocated for the task. We believe this was due in part to problems with the documentation search engine and insufficient sample coverage; however, it is clear that the API itself had several usability problems. If we had conducted the studies before shipping the product, we could have eliminated a significant source of customer dissatisfaction and avoided the cost of trying to fix the API of a major feature area without introducing breaking changes.


BRAD ABRAMS There is no more powerful experience to give an API designer a visceral understanding of the usability of his or her API than sitting behind a one-way mirror watching developer after developer get frustrated with an API he or she designed and ultimately fail to complete the task. I personally went through a full range ofemotions while watching the usability studies for System.IO that we did right after shipping version 1.0. As developer after developer failed to complete the simple task, my emotions moved from arrogance to disbelief, then to frustration, and finally to stringent resolve to fix the problem in the API.


как бы намекает, что так делать не надо
Re[13]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 21.09.16 10:11
Оценка: +1
Здравствуйте, Sinix, Вы писали:

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



S>>>Поспрашивал, твой вариант части народа понравился, остальным не понравился Intersects — типа работа у нас с enum-ами, делаем для типовых программистов, требовать от всех уметь в множества — издевательство.

S>>Тут мое мнение по части типового программиста расходится с остальнымм народом (кому не понравился Intersects).

S>Моё, если честно, тоже. Но вот это правило

S>с комментариями от людей, уже наступавших на эти грабли


S>как бы намекает, что так делать не надо


Правило и комментарии тут обоюдоострые. Они же (в совокупности с моим примером о том, что я не понял, что делает IsAnyFlagSet) говорят что и так делать тоже не надо. А с моим случаем еще хуже. Ты мне объяснил, но я уже забыл, что делает метод. И глядя на название — не могу вспомнить. Глядя в код, скорее всего, понял бы, но удивился бы.
В то же время IsDiffNotEmpty не оставляет вообще никаких шансов на непонимание.

Кстати, у Enumerable.Except() в хелпе написано "Produces the set difference of two sequences". Не противоречит правилу и комментариям к нему?
Re[14]: EnumHelper: request for breaking changes
От: Sinix  
Дата: 21.09.16 10:18
Оценка:
Здравствуйте, samius, Вы писали:

S>Кстати, у Enumerable.Except() в хелпе написано "Produces the set difference of two sequences". Не противоречит правилу и комментариям к нему?

Точка зрения принята

Вариант с Includes/IncludesAny/Excludes/ExcludesAny в принципе пройдёт, но тогда надо придумать нормальное название для SetFlag(value, enabled: false);
Чуть позже заведу тему в основном разделе, пусть народ поучаствует. Найдётся вариант лучше — будем менять.
Re[15]: EnumHelper: request for breaking changes
От: samius Япония http://sams-tricks.blogspot.com
Дата: 21.09.16 11:02
Оценка:
Здравствуйте, Sinix, Вы писали:

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


S>Вариант с Includes/IncludesAny/Excludes/ExcludesAny в принципе пройдёт, но тогда надо придумать нормальное название для SetFlag(value, enabled: false);

Тут сложно придумать что-либо соответствующее теории множетсв, разве что .Update(value, enabled) или .Combine(value, enabled).
еще пару замечаний. value — это не value, а множество флагов, которые требуется установить/снять/обновить. enabled — хорошо бы был true по умолчанию, что бы SetFlag без указания enabled не сносил, а устанавливал флаги. Но это касается именно SetFlag.

S>Чуть позже заведу тему в основном разделе, пусть народ поучаствует. Найдётся вариант лучше — будем менять.

Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.