Здравствуйте, Буравчик, Вы писали:
Б>Polymorphism Б>Say we have the method UtilityClass.SomeMethod that happily buzzes along. Suddenly we need to change the functionality slightly... Had it not been a static method, we could make a derivate class and change the method contents as needed.
Потрясающе! Взяли и на ровном месте создали сильнейшую (наследование) связь между двумя классами! Теперь родителя стало менять сложнее. Нужно сохранять вновь созданный контракт. Нельзя изменить поведение метода, нельзя избавиться от метода, нельзя заменить класс на композицию из нескольких более мелких классов. А ведь можно было выделить интерфейс для нужной связи и параметризовать класс этим поведением. Или сделать еще один статический метод, в котором создать того наследника, без его публичной видимости. Второй вариант не исключает первого — можно вместо наследника передавать функцию/поведение даже без открытия такой возможности вовне. Похоже, автор владеет очень маленьким набором возможных приемов создания API. Поэтому и получаем решение из серии "если у вас в руках микроскоп, все вокруг кажется гвоздями".
Б>Interface woes Б>Testing
Это не проблема класса со статическими методами. Это вообще проблема класса, который использует интерфейсы. И порядок разаботки здесь предполагает, что сначала написан пользователь и интерфейс, а потом пишется реализация интерфейса. Кроме того, как это все поможет, если клиент явно создает экземпляр класса для вызова метода? А если не создает, то почему бы и не вызвать статический метод из одной из реализаций интерфейса зависимости?
Б>Fosters blobs Б>As static methods are usually used as utility methods and utility methods usually will have different purposes, we'll quickly end up with a large class filled up with non-coherent functionality — ideally, each class should have a single purpose within the system. I'd much rather have a five times the classes as long as their purposes are well defined.
Тоже прекрасно. Ну вот кто, кто мешает раскидать статические методы по нескольким статическим классам? И никто при этом не мешает накидать кучу слабосвязных методов в один такой "утилитный экземпляр". Это проблемы разработчиков, которые не хотят думать, куда же относятся их API и сваливают их в одно место.
Б>Parameter creep Б>Had we created a class with the single purpose of doing what this static method did, we could solve this by taking in the required parameters in the constructor, and allowing the user to set optional values through properties, or methods to set multiple interdependent values at the same time.
Шаблон parameter object давным давно придумали. Точно так же можно все сделать. В современных языках есть еще параметры по-умолчанию. Кроме того, parameter object позволяет лучше выражать сложные ограничения:
Я явно указываю, что если указан параметр 4, что требуется указать еще параметры 6 и 7. Как автор предлагает делать подобное ограничение с его же геттерами и сеттерами?
Б>Also, if a method has grown to this amount of complexity, it most likely needs to be in its own class anyways.
Еще более вероятно, что там будут задействованы несколько классов. И вот как раз с отдельным статическим методом настраивать их взаимодействие будет гораздо проще.
Б>Demanding consumers to create an instance of classes for no reason Б>Adding an extra line... simply create a singleton wrapper..., you can still create static wrapper..., you could also make a class that hides the instantiation as if it was a singleton: MyWrapper.Instance is a property that just returns new MyClass();
Не было бабе заботы — купила баба порося. Насоздавали себе проблем, теперь с ними нужно героически бороться.
Б>Only a Sith deals in absolutes Б>If your project is a one-off with no requirements for future maintenance, the overall architecture really isn't very important — static or non static, doesn't really matter — development speed does, however.
Так вот как раз статические методы лучше для maintenance. Они являются очень сильной (и при этом простой) границей между компонентами приложения и минимизируют шанс, что где-то в отдаленной части приложения что-то полагается на конкретные детали реализации нашего класса (привет, наследование!). Эти же методы лучше для тестирования — методы тестируются независимо. Клей (зависимости для DI) живут вообще отдельно от всех этих методов.
Здравствуйте, maxkar, Вы писали:
M>Тоже связано с тем, что с абстракциями не очень (правда уже применительно к процессу разработки). Не факт, что там вообще было промежуточное работающее остояние, которое можно было бы закоммитить. А может быть он из среды, где все коммиты сливаются в один при мерже, так зачем стараться?
Ну вот хз, лично я коммичу часто. Даже в самом начале проекта. Основной критерий — чтоб компитировалось, что не работает и до хрена TODO пофиг. Да и не на начальном этапе тоже, по умолчанию использую trunk based development с feature flags и вполне никому не мешаю и редко что ломаю. Хотя я в курсе что мало кто так делает, про git flow я знаю. При желании могу и красиво и правильно делать, как положено, что прям историю почитаешь и душа радуется. Но без необходимости заморачиваться нет желания.
M>Все вышенаписанное вообще должно делаться автоматически. Это стандартные проблемы дизайна со стандартными решениями и причинами таких решений. Там даже мозг можно не включать!
Именно! Я в свое время даже давал тестовое задание с ограничением по времени (час) на проверку рефлексов. Народ хреначил сутки и выдавал такую мешанину фабрик фабрик и применял все паттерны проектирования, при этом не расширяемо ни как. Через определенное время возникли сомнения и я прям параллельно с кандидатом с 23 часа ночи решил сам сделать свое тестовое задание. Уложился за 30 минут, не смотря что уставший был в усмерть, хотя там работы вообще на 10 минут максимум. Потом сам свое эталонное решение показываю кандидату — прям откровение было. Откровение что можно оказывается не городить классы и фабрики, а тупо на статических методах быстро сделать поддерживаемое расширяемое решение с нужными уровнями абстракции, которые крайне легко заменять.
И это не означает что я всегда только пишу на статических методах, иногда я вполне использую самое настоящее ООП, прям эталонное. То когда требуется, я не ООП ради ООП.
Здравствуйте, Sharov, Вы писали:
S>Я бы так не сказал, учитывая что после видеодрайвера работу начнет соотв. железо, т.е. это будет быстро и по памяти не очень много сожрет ( хотя зависит от). S>А вот множество new и по времени и по памяти да еще + GC могут дорого обойтись.
Я имел в виду один или несколько лишних new на фоне рисования GUI, в каком-нибудь OnPaint. А так-то да, конечно.
Здравствуйте, maxkar, Вы писали:
M>А может быть, реализация имеет высокую связность с классом пользователя и мы ее сделаем в том же классе реализации. Декораторами интерфейса можно добавить всякие красивости в виде логирования и метрик (и их можно потом использовать с другими реализациями логики калькулятора).
А можешь написать пример с декоратором если не сложно?
Здравствуйте, Kernan, Вы писали:
M>>А может быть, реализация имеет высокую связность с классом пользователя и мы ее сделаем в том же классе реализации. Декораторами интерфейса можно добавить всякие красивости в виде логирования и метрик (и их можно потом использовать с другими реализациями логики калькулятора). K>А можешь написать пример с декоратором если не сложно?
Могу. Было:
public interface MyCalculator {
public Result calculate(Input input);
}
public class BasicCalculator implements MyCalculator {
@override
public Result calculate(Input input) {
return Calculator.calculate(input);
}
}
Теперь мы добавляем логирование (логгер инжектится, потому что глобальные переменные — зло ):
/**
* An implementation of the calculator that logs input and output of the operations performed
* by another implementation of the calculator.
*/public final class LoggingCalculator implements MyCalculator {
private final MyCalculator peer;
private final Logger logger;
/**
* Creates a new calculator.
* @param peer the actual calculation logic.
* @param logger logger to write information about the calls.
*/public LoggingCalculator(MyCalculator peer, Logger logger) {
this.peer = peer;
this.logger = logger;
}
@override
public Result calculate(Input input) {
logger.info("Calling the calculate method with input {}".format(input));
try {
final Result res = peer.calculate(input);
logger.info("Got the result of {}".format(res));
return res;
} catch (RuntimeException e) {
logger.error("Internal error on calling calculate with the input {}".format(input), e);
throw e;
} catch (Error e) {
logger.fatal("Fatal error calling calculate with input {}".format(input), e);
throw e;
}
}
}
Метрики:
/**
* An implementation of calculator that collects some basic stats about the calls to the calculation method.
* Use `metrics` of this instance to get access to the metrics exporter.
*/public final class MetricsCalculator implements MyCalculator {
private final MyCalculator peer;
private final long slowpokeLimit;
private final AtomicLong startCount = new AtomicLong();
private final AtomicLong successCount = new AtomicLong();
private final AtomicLong failureCount = new AtomicLong();
private final AtomicLong slowpokeCount = new AtomicLong();
/**
* Creates a new calculator that collects basic metrics.
* @param peer actual calculation logics (for which metrics are collected).
* @param slowpokeLimitMs time limit (in milliseconds) after which the calculation will be
* marked as "slow". "Slow" calls contribute to both successful and slow operation counters.
*/public MetricsCalculator(MyCalculator peer, long slowpokeLimitMs) {
this.peer = peer;
this.slowpokeLimit = slowpokeLimitMs;
}
public final MetricsExporter metrics = new MetricsExporter() {
@Override
public void writeTo(MetricsWriter writer) {
writer.addValue("started", startCount.get());
writer.addValue("successes", successCount.get());
writer.addValue("failures", failureCount.get());
writer.addValue("slowpokes", slowpokeCount.get());
}
}
@override
public Result calculate(Input input) {
startCount.incrementAndGet();
final long startTime = System.currentTimeMillis();
try {
final Result res = peer.calculate(input);
final endTime = System.currentTimeMillis();
successCount.incrementAndGet();
if (endTime - startTime > slowpokeLimit)
slowpokeCount.incrementAndGet();
} catch (RuntimeException e) {
failureCount.incrementAndGet();
throw e;
} catch (Error e) {
failureCount.incrementAndGet();
throw e;
}
}
}
Как-то так. Оно не очень дружит со всякими DI-фреймворками. По типам декораторы не разруливаются, явное прописывание имен/связей в классах нарушает SRP. Зато все очень хорошо ложится в ручной код старта. Там можно навешивать/удалять декораторы (не нужно менять реализацию декоратора!). Или реализовать логику навешивания декораторов на основе конфигурационных файлов, параметров командной строки и чего угодно. Что еще? Например, можно написать A/B-комбинаторы (которые будут два peer получать). Например, делегировать какую-то часть вызовов к новой реализации, потом сравнивать результат. В этом случае логгеры/метрики можно навешивать только на интересующую нас ветку. Именно из-за возможности нескольких реализаций логгер передается как параметр — может быть желательно различать две этих реализации в логах. Для метрик идентификация/различение делается при построении результирующего экспортёра из нескольких мелких экспортёров.
Все эти метрики и логи не имеют видимости "внутрь" вызываемого метода (т.е. в Calculator.calculate). Но для алгоритма это обычно не нужно. Логика должна быть более-менее оттестирована и надежна. Если вдруг хочется логи добавить внутри, значит нет полной уверенности в корректности и нужно сделать тесты, провести ревью и т.п. Плюс логирующий декоратор на границе может быть достаточен для поиска большинства проблем. Он пишет входные данные, так что в случае исключения можно будет взять эти данные и написать тест. Быстродествие для алгоритмов тоже обычно делается "в песочнице" — синтетические данные и синтетические замеры. Какие-то особенности входных данных (распределение по объему и т.п.) тоже во многих случаях можно собрать декораторами.
В крайнем случае все-таки придется изменить API, избавиться от статического метода и сделать его методом экземпляра. Но там в большинстве случаев будут нужны и какие-то другие шаги по интеграции. Например, передать логгер или интегрировать систему с подсистемой метрик. Так что старый код вида "new Calculator().calculate()" все равно не работал бы. В худшем случае пришлось бы тащить лишние зависимости в клиента, где этот код. А так нам достаточно изменить реалиацию BasicCalculator в которой все изолировано. В лучшем случае она станет получать экземпляр уже сконфигурированного Calculator. В худшем — получать его параметры и создавать новый экземпляр на каждое вычисление. Но даже в этом случае вся "грязь" оказывается изолирована в одном месте.
Здравствуйте, Pavel Dvorkin, Вы писали:
S>>Я бы так не сказал, учитывая что после видеодрайвера работу начнет соотв. железо, т.е. это будет быстро и по памяти не очень много сожрет ( хотя зависит от). S>>А вот множество new и по времени и по памяти да еще + GC могут дорого обойтись. PD>Я имел в виду один или несколько лишних new на фоне рисования GUI, в каком-нибудь OnPaint. А так-то да, конечно.
Но даже тут не факт, если OnPaint будет использовать gpu, а new будет с наследниками + каскадные new какие-нибудь.
Здравствуйте, elmal, Вы писали:
E>Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров.
Может, атавизм развития кода в процессе написания?
Изначально подобную технику используют, когда перед вызовом неких вычислений параметры настраивают в несколько шагов.
Может быть так, что надобность в многошаговой настройке и вообще настройке отпала, а выделенный класс остался.
Может, стоило просто указать, что вот тут неподметённый атавизм и забыть про вопрос?
E>Но мне вот интересно, это я такой непродвинутый и это только меня напрягает?
Товарищ уже понял правду жизни, что "юнитами теста" и "применением паттернов" не занимаются практически нигде кроме собеседований.
Здравствуйте, Sinclair, Вы писали:
S>Возможно, там вызов calculate проинлайнился, и вообще вся инициализация выбросилась JIT-ом вместе с экземпляром, и даже регистр этой переменной не занят.
А такое бывает? Без стёба — просто спрашиваю. Я плотно работал только с CLR 4 — там это так не работает. С последними версиями шарпа не работал.
Всё сказанное выше — личное мнение, если не указано обратное.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>И тут автор начинает задумываться. А что , если со временем этот процесс калькуляции будет изменен ? Если в нем будет не простенькая формула, а несколько стадий, с передачей данных между ними ? Передача данных и static — это вещи почти несовместимые, в многопоточном приложении нарвешься на неприятности. Нет уж, лучше я сделаю класс и экземпляр, тут я твердо уверен, что то самое внутреннее состояние, о котором ты пишешь, мне легко сделать зависящим от него только и ни от чего больше. Добавляю я туда несколько полей, реализую логику вычислений по иному — никто, кроме этого экземпляра и знать не будет.
В таких случаях делают фабрику, или как минимум фабричный метод. Тогда убивается сразу 2 зайца: возможность замены реализации и возможность тестирования: замена всяких "calculate" при написании юнит-тестов — регулярная задача. Притом не важно как реализована фабрика или фабричный метод — важно что они есть и их можно переопределить.
new по месту, да ещё и в цикле — просто явный признак того, что человек никогда не занимался оптимизацией.
Будь там вот такое, то это был бы совсем другой разговор.
for (;;)
{
caclulator = GetCaclulator();
caclulator.Calculate(data);
}
Всё сказанное выше — личное мнение, если не указано обратное.
Здравствуйте, elmal, Вы писали:
E>... Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Это ж тестовое задание. Глупость само по себе — проверяет только то, как человек сделает написанное на бумажке задание за минимум времени. Неизвестно, как автор будет писать в рамках кодовой базы вашей конторы. Может, адаптируется и всё будет норм, а может, уволите через пару месяцев. Все это еще и сильно зависит от того, насколько в вашей конторе ценится код. Судя по тому, что ты пишешь о "практиках" — очень ценится, но это совсем не везде так.
vaa>придираетесь)) vaa>жестянщик не обязан уметь строить ракеты как ракетчик, хотя оба работают с материалами. vaa>скорость проф роста также зависит от способностей, возможностей, приоритетов.
Оная скорость еще чрезвычайно сильно зависит от наличия строгого, но справедливого руководителя, который делает "трах ин дер поппен" за плохую работу и заставляеет переделывать, и в то же время хвалит за хорошую. Самоучки, к сожалению, такого мощного катализатора роста лишены.
Я по себе, блин, замечаю: в свое время мне повезло попасть к такому человеку, и через годик я начал писать под iOS/Mac настолько структурированно, что чужие портянки кода с гитхаба, и тем более корявые официальные "Apple examples", стали вызывать приступы снобизма и прихихикивания: "ну, блин, и говнокодеры".
Но стоило мне уйти в другое место, где во главу угла поставлена была скорость закрытия заданий в Джире, а на качество смотрели сквозь пальцы, то сразу, к сожалению, начал писать халтурно и как все. Потому что красивый код более трудоемок, а если эта дополнительная трудоемкость не предусмотрена в общем процессе, то увы и ой.