а что тут такого? Предлагаете писать фабрику для каждого типа? Или может мапы строить и потом по рефлексии создавать колонку? Всё одинаково плохо для такого простого примера. KIS как он есть.
Ну и какой в этом смысл. Фабрика нужна, если набор динамический. А для статического набора — ну единственный минус, такое линейный поиск имеет сложность O(N), а хеш-таблица имеет сложность O(1). Но скорей всего реально разницы не будет.
Здравствуйте, sergii.p, Вы писали:
SP>а что тут такого? Предлагаете писать фабрику для каждого типа? Или может мапы строить и потом по рефлексии создавать колонку?
Ну, хотя бы так:
private static DataFrameColumn CreateColumn(Type kind, string[] columnNames, int columnIndex)
{
var colName = GetColumnName(columnNames, columnIndex);
return kind switch
{
typeof (bool) => new BooleanDataFrameColumn(colName),
typeof (int) => new Int32DataFrameColumn(colName),
...
_ => throw new NotSupportedException()
};
}
4>однократно прописать перед блоком if-ов, на производительность не повлияет, но будет меньше копипасты.
От этого поведение чуток измениться, если GetColumnName(columnNames, columnIndex) выдаст исключение, оно трегернет первым до проверки типа колонки. Причем уже в другом методе.
Вероятно это и послужило причиной того что сначала проверка, а потом уже GetColumnName
Здравствуйте, Codealot, Вы писали:
C>Всякое я раньше видел, но это как-то особенно пробрало.
Я не понял... тебя "пробрало", но при этом ты тупо фэйспалмишь, ничего не объясняя. И главное, не предлагая правильной альтернативы! Походу ты сам пока джун и увидел тупо простыню кода и считаешь, что это неправильно. Хотя после 10 лет ковыряния в "интыпрайзе" и чёрт-ногу-сломит фабриках и "шаблонах четырёх", ты придёшь к одному выводу — "чем проще выглядит код, тем лучше". Лучше для сопровождения, для отладки, для объяснения другим, для документации и т.п.
В принципе, этот код неплох, но я бы сделал чутка короче: словарь "тип -> конструктор колонки" и далее constr[type].Invoke... (ну или Activator.CreateInstance).
else
{
throw new NotSupportedException(nameof(kind));
}
Это ублюдство работает до поры до времени, пока прогер сидит локально в CreateColumn(Type kind, string[] columnNames, int columnIndex) и ему "всё понятно". Но когда исключение вылезет эдак на 10 уровней выше, удивлённая индусятина обнаружит "not supported ComplexType" и даже не поймёт, кто кого и зачем должен поддерживать. А ведь ИМЕННО ЭТИ ошибки вылезают у юзера в самый ненужный момент и всё, что предлагает микрософт — "отошлите нам годичную телеметрию, МЫ САМИ НИ ЧЕРТА НЕ ПОНИМАЕМ В СВОЁМ КОДЕ!".
Ну так вы и не понимаете именно в силу вот таких new NotSupportedException!!
Ошибки всегда надо сопровождать локльным контекстом и/или кратким адекватным комментом — "при создании колонки использован неизвестный тип ДаОткудаТыВзялсяТварь".
Собственно, MS потому и прячет всё своё говно под коврик, что они сами не понимают, что за проблема у юзера. Писали бы они нормальные комменты к иксепшынам, распоследний юзер мог бы им НОРМАЛЬНО рапортовать: "ребят, вот такой месыдж, чините!", а не отправлять мегабайты телеметрии, чтобы обезьяна копалась весь день в логах. Аштрисёт!! (ц)
А чем этот код плох?
1) Можно было был if превратить в switch, но видимо код был написан до появления этой фичи в языке.
2) Заменить if\switch на "фабрики" или просто таблицу — сильно просадит перформанс. Тем более сейчас мэтчинг оптимизирован под популярные типы (мы же знаем что переходы вперед предсказываются как ложные).
3) Тип эксепшена не тот — это не имеет значения на самом деле. Это usage\design exeptions, их не надо ловить. Достаточно чтобы эксепшен сказал что с кодом не так.
Остается только GetColumnName не надо вызывать в каждой ветке, можно вызвать до if. Скорее всего тупо копипаста, в целом неопасная, так как ни вызов GetColumnName, ни его содержимое меняться не будет.
Здравствуйте, Ночной Смотрящий, Вы писали:
НС>Ну, хотя бы так:
Этот код, небось, копируется со времен царя Гороха, когда switch выглядел немногим лучше if else if
Здравствуйте, Ночной Смотрящий, Вы писали:
SP>>а что тут такого? Предлагаете писать фабрику для каждого типа? Или может мапы строить и потом по рефлексии создавать колонку? НС>Ну, хотя бы так:
Здравствуйте, sergii.p, Вы писали:
SP>а что тут такого? Предлагаете писать фабрику для каждого типа? Или может мапы строить и потом по рефлексии создавать колонку? Всё одинаково плохо для такого простого примера. KIS как он есть.
Предлагаю помнить про DRY и не копипастить GetColumnName(columnNames, columnIndex) в каждую строку. Да и вообще идея тащить в метод весь массив имен, где он нафиг не нужен — она очень странная, мягко говоря.
Странно, что такие азы надо объяснять.
Здравствуйте, Teolog, Вы писали:
T>если GetColumnName(columnNames, columnIndex) выдаст исключение, оно трегернет первым до проверки типа колонки. Причем уже в другом методе.
Здравствуйте, sergii.p, Вы писали:
SP>а что тут такого? Предлагаете писать фабрику для каждого типа? Или может мапы строить и потом по рефлексии создавать колонку? Всё одинаково плохо для такого простого примера. KIS как он есть.
Для примитивных типов, которые там примерно все, можно использовать Type.GetTypeCode.
Если нам не помогут, то мы тоже никого не пощадим.
Если нам не помогут, то мы тоже никого не пощадим.
