Здравствуйте, elmal, Вы писали:
E>Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар.
Причина та же, что и у оригинального вопроса про класс — слабые навыки выделения абстракций. И остутствие инструментария для работы с ними.
E>Все отправлено в виде одного коммита в GIT, и не видно истории как все делалось — допустим тоже так и надо.
Тоже связано с тем, что с абстракциями не очень (правда уже применительно к процессу разработки). Не факт, что там вообще было промежуточное работающее остояние, которое можно было бы закоммитить. А может быть он из среды, где все коммиты сливаются в один при мерже, так зачем стараться?
E>Но мне вот интересно, это я такой непродвинутый и это только меня напрягает?
Меня такое тоже напрягает. Потому что смысла там мало (кстати, комментариев/документации к классу и методу тоже отсутствуют?). Смотрите, как это делается.
Допустим, мы решили, что какую-то функциональность нужно вынести из места использования (т.е. вызывающего класса) куда-то извне. Потому что она сложная и/или полезная. Фунцкиональность у вас без побочных эффектов. Поэтому API у нее будет статическим, это обеспечивает меньшее зацепление (coupling) между клиентами и реализацией. Например,
public class Calculator {
private Calculator() {
throw new UnsupportedOperationException("Don't even try");
}
public static Result calculate(Input input) {
...
}
}
Там делаем вычисления. В ряде случаев бывает удобно иметь внутреннее состояние на время вычисления. Всякие парсеры могут входные данные и позицию разбора разделять между многими методам и т.п. Но это детали реализации. Поэтому мы можем сохранить API и изменить реализацию:
public class Calculator {
private final Input input;
private Calculator(Input input) {
this.input = input;
}
private Result calculateImpl() {
...
}
public static Result calculate(Input input) {
final Calculator calc = new Calculator(input);
...
return calc.calculateImpl();
}
}
Это может быть один метод к calculate, может быть — много вызовов. Со временем код может рефакториться дальше. Например, в процессе работы образовалось несколько удобных утилитарных классов и вычисления можно сделать через них. В этом случае статический метод будет изменен на создание/вызов этих новых утилит вместо калькулятора. Прежняя реализация будет не нужна и может быть выкинута. С классом это все будет выглядеть гораздо менее естественно.
Еще плюсы. В статических методах можно делать всякую предобработку. Например, преобразовать Input с использованием стандартной библиотеки и передать в конструктор Calculator то, что ближе ему в рамках реализации (т.е. более cohesive). Например, передать tokenStream вместо inputStream или что-то подобное. А иначе конструктор класса превращается в что-то менее потребное.
С отдельным методом можно производить более плавную эволюцию. Например, у нас добавили дополнительные настройки. Мы можем добавить новый метод, который принимает параметр. Потом вызвать новый метод из старого, задепрекейтить старый и постепенно (по-одному) заменить все использования со старого на новый. Можно еще тесты гонять по желанию после каждого изменения, чтобы знать, что конкретно поменялось. Т.е. будет у нас что-то вроде
public class Calculator {
private final Input input;
private Calculator(Input input) {
this.input = input;
}
private Result calculateImpl() {
...
}
@Deprecated("use calculate(input, Options.defaultOptions())")
public static Result calculate(Input input) {
return calculate(input, Options.defaultOptions());
}
public static Result calculate(Input input, Options options) {
if (!options.useFeatureA() && !options.useFeatureB())
return new Calculator.calculate();
DataStream ds =
options.useFeatureA() ? new FancyStream(input.stream()) : input.stream();
return new BetterImpl(options.useFeatureB().process(ds);
}
}
Подобные вещи нормально смотрятся в функции. Через класс их тоже можно таскать. Но очень часто при подобной эволюции ответственность класса размывается. Он перестает быть сфокусирован на одной конкретной задаче и начинает работать с совершенно разными уровнями абстракций.
Если хочется DI, то это совершенно другая проблема, которая решается по месту использования. Опять же, из протокола обмена у нас ровно один метод. Он точно так же описывается интерфейсом с одним методом (нет, не фабрикой!). Этот интерфейс идет в тот же пакет/неймспейс, что и его пользователь. Он ближе по происхождению к пользователю. И изменяется он (интерфейс) в основном с пользователем. Т.е.
public interface MyCalculator {
public Result calculate(Input input);
}
class MyProg {
private final MyCalculator calc;
MyProg(MyCalculator calc) {
this.calc = calc;
}
void doSomething(Input input) {
for (int i = 0; i < 10; i++) {
calculator.calculate(input);
}
}
}
Потом мы пишем реализацию. Тут могут быть варианты. Если у нас уже есть "внешняя" реализация — можем просто делегировать к ней:
public class BasicCalculator implements MyCalculator {
@override
public Result calculate(Input input) {
return Calculator.calculate(input);
}
}
А может быть, реализация имеет высокую связность с классом пользователя и мы ее сделаем в том же классе реализации. Декораторами интерфейса можно добавить всякие красивости в виде логирования и метрик (и их можно потом использовать с другими реализациями логики калькулятора). Кстати, предыдущий код калькулятора не очень — сам тип BasicCalculator не несет никакой дополнительной функциональности (нет другого API кроме MyCalculator). Поэтому можно его создавать методом-конструктором. Или, в данном случае, статическим полем:
final class MyCalculators {
private MyCalculators() {
throw new UnsupportedOperationException("Why do you want this?");
}
/**
* A basic implementation of the calculator that just delegates all
* computation to the standard library implementation Calculator.
*/public MyCalculator basicCalculator =
new MyCalculator() {
@override
public Result calculate(Input input) {
return Calculator.calculate(input);
}
}
}
Т.е. мы рассмотрели несколько задач:
Какой API используется для доступа к функциональности, предоставляемой модулем вычисления?
Как реализуется функциональность, предоставляемая модулем вычисления?
Какой API требуется для параметризации работы нашего алгоритма?
Как реализовать требуемый функционал в нашем конкретном контексте использования?
Да, в какой-то мере задачи связные. Но хороший API при этом позволяет очень сильно изменять реализацию внутри. И очень часто при необходимости изменения API позволяет это сделать по-шагам. А еще, как вы можете заметить, у нас получились натуральные места, где можно сделать коммиты.
Все вышенаписанное вообще должно делаться автоматически. Это стандартные проблемы дизайна со стандартными решениями и причинами таких решений. Там даже мозг можно не включать!
И вот здесь мы опять возвращаемся к одному большому коммиту гит. Так и получается, что очень много разработчиков не знают "технологический процесс" (кто там недавно писал про технологические карты? ). Они не видят возникающих в процессе написания стандартных вопросов и решений. Не могут выделить один или несколько "тесно связанных" задач, которые составят прекрасный полностью законченный коммит (такое не всегда возможно, но у вас в примере — вполне).
Здравствуйте, elmal, Вы писали:
E>Там нет n локальных переменных. Там ровно один параметр на входе и один на выходе, после чего никакое внутреннее состояние объекта не интересно. По существу там чистая функция, и что то я сомневаюсь что Мартин бы такое посоветовал. static метод определенного класса полностью бы решил все проблемы (пусть даже и с созданием экземпляра внутри, ибо там Thread Safe требуется).
Тогда похоже на типичные симптомы синдрома Александреску.
Здравствуйте, 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. В худшем — получать его параметры и создавать новый экземпляр на каждое вычисление. Но даже в этом случае вся "грязь" оказывается изолирована в одном месте.
Здравствуйте, Буравчик, Вы писали:
Б>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) живут вообще отдельно от всех этих методов.
Здравствуйте, elmal, Вы писали:
E>Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Идеологически — создание нового экземпляра класса верно и оправданно. Если мне нужна новая табуретка, то надо сделать новую табуретку, а не брать табуретку, которую я сделал год назад и пытаться ее переделать под требования к новой табуретке.
Практически — ужасающе, конечно. Тем более, если я правильно понял, там класс совсем несложный, и static экземпляр решил бы все проблемы. Либо создать до цикла.
До поры до времени никто в цикле и не стал бы делать, разве что совсем уж начинающие. Памяти было мало , удаление ручное, быстродействие маленькое.
Ну а сейчас всем объяснили, что память не ресурс, что GC все сделает, что предварительная оптимизация зло и т.д. В общем, не парьтесь, делайте так, а если выяснится потом, что это и есть bottleneck, то тогда и отрефакторите.
Увы, это стало почти что нормой и объяснить , что это плохо, очень трудно.
Здравствуйте, elmal, Вы писали:
E>Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Отдельный класс подразумевает разделение области ответственности, что зачастую ожидается и верно.
Возможно, его следовало сделать статиком, если никакого состояния не предполагалось.
Здравствуйте, elmal, Вы писали:
E>Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Норм. Разработчик реализовал алгоритм и инкапсулировал его в класс — нормальная практика.
Это особенно удобно, если логика сложная (разбивается на несколько функций) и/или логика требует некоторой параметризации (передается в конструктор).
Минусы статиков по сравнению с обычным классом:
— если логика сложная — то все вспомогательные функции тоже должны быть статик
— если появятся параметры класса, то придется все статики переделать в нормальные методы (а параметры обязательно появятся)
— код прибит к классу — никакого тестирования, никакого IoC
Обычный (не статик) класс легко превратить в статик — просто создав единственный экземпляр класса (ага, синглетон), и обращаться к нему.
Это тоже не очень хороший вариант, но он имеет меньше перечисленных выше проблем.
P.S. Ну и в целом, класс с одним публичным методом — нормальная практика при использовании IoC и DI
ДОБАВЛЕНО:
Понятно, что для "простейшего случая" это все может быть оверхедом — ведь все зависит от ситуации, от требований. Если случай был уж совсем простой, и можно было ограничиться одной простой чистой функцией, то предлагаю считать, что разработчик просто предомонстрировал в тестовом задании умение работать с таким паттерном (инкапсуляция алгоритма в класс). Тоже имхо не плохой вариант.
Ну у меня такое обычно получается в процессе рефакторинга, т.е.
static int Calculate(int x);
сначала вырастает в:
static int Calculate(int x, int y, int z, double a, out string c);
потом разбивается на:
static int Calculate(int x, int y, int z, double a, out string c)
{
int tmp1 = CalculateSomething(x, y, z);
int tmp2 = CalculateSomethingElse(tmp1, z, a);
c = CalculateSomeAuxiliaryOutput(tmp1, tmp2);
return tmp2;
}
и потом, чтобы не засорять область видимости временными CalculateSomething(), и чтобы не передавать вручную по 10 параметров, упрощается в:
class Calculator
{
readonly int X, Y, Z, A;
public string C{get; private set;}
public Calculator(int x, int y, int z, double a) {...}
private int CalculateSomething();
public int Calculate()
{
int tmp1 = CalculateSomething();
int tmp2 = CalculateSomethingElse(tmp1);
CalculateSomeAuxiliaryOutput(tmp1, tmp2);
return tmp2;
}
}
Если Calculate() хоть сколько-либо нетривиально, лишнее выделение памяти здесь погоды не сделает.
Здравствуйте, elmal, Вы писали:
E>Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
мне кажется, тут ноги растут из просчёта языка, что нельзя создать свободную функцию. Вот и приходится добавлять "классы ради классов" с функциональностью областей видимости (namespace). А то как оно вызывается — абсолютно незначимый момент.
Я даже не понимаю зачем это ограничение до сих пор в стандарте. Можно было уже сотню раз раскаяться и не плодить костыли вроде static class.
vaa>придираетесь)) vaa>жестянщик не обязан уметь строить ракеты как ракетчик, хотя оба работают с материалами. vaa>скорость проф роста также зависит от способностей, возможностей, приоритетов.
Оная скорость еще чрезвычайно сильно зависит от наличия строгого, но справедливого руководителя, который делает "трах ин дер поппен" за плохую работу и заставляеет переделывать, и в то же время хвалит за хорошую. Самоучки, к сожалению, такого мощного катализатора роста лишены.
Я по себе, блин, замечаю: в свое время мне повезло попасть к такому человеку, и через годик я начал писать под iOS/Mac настолько структурированно, что чужие портянки кода с гитхаба, и тем более корявые официальные "Apple examples", стали вызывать приступы снобизма и прихихикивания: "ну, блин, и говнокодеры".
Но стоило мне уйти в другое место, где во главу угла поставлена была скорость закрытия заданий в Джире, а на качество смотрели сквозь пальцы, то сразу, к сожалению, начал писать халтурно и как все. Потому что красивый код более трудоемок, а если эта дополнительная трудоемкость не предусмотрена в общем процессе, то увы и ой.
ЮЗдравствуйте, elmal, Вы писали:
E>Однако как создавать? Если брать табуретку, то идеологически наверно у нас есть фабрика табуреток, которой мы скармливаем необходимые материалы и говорим сделай табуретку. И фабрика табуреток у нас одна и нам не нужно для каждой табуретки сначала стрить фабрику.
Ну этот вопрос спорный. Как городской житель я, конечно, закажу табуретку фабрике. Но сельский житель вполне возможно сделает ее сам. В прошлом году жена пристала — сделай табуретку. Сделал ей, квадратную. Так она в каком-то журнале увидела круглую табуретку и теперь требует такую. Квадратная ей больше не нужна. Пытаюсь сделать из квадратной круглую — что-то плохо получается .А фабрики в деревне нет.
>Здесь же самое близкое — это мы создаем объект табуретка и у этого объекта дергаем метод создайся. А на деле там еще веселее. Нужно, условно говоря, посчитать значение какой формулы. На входе выражение, на выходе число. Логически с точки зрения использования одна функция, причем чистая функция.
Верно, чистая функция. Статический метод класса, если о Java речь идет
>Но практически создают новый экземпляр класса интерпретатора выражений с одним методом "вычислить". Хорошо хоть не додумались этому интерпретатору выражений сначала посылать сообщение "получи входные данные", потом сообщение "посчитай", а потом посылать сообщение отдай "результат". А то ведь и до такого можно было додуматься в теории.
Ну вообще-то небольшой нюанс тут есть. На static можно нехило проколоться в многопоточном приложении. Добавишь статическое поле — и привет.
E>Я просто несколько удивился, что можно проработать в индустрии более 10 лет, на разных работах, на буржуйских заказчиков, и при этом за подобное не получать по рукам и не делать по умолчанию наиболее удобно к использованию.
Все зависит от того, чем именно он занимался.
Где-то такое приведет к приличной просадке скорости или проблемам с памятью. А где-то ничего заметного не будет. Например, в GUI — пока приложение вызовет ОС для отрисовки, а ОС перебросит все это видеодрайверу, а тот будет думать, как это лучше отрисовать, потому что у него таких приложений много, а потом начнет шрифты рендерить.. На этом фоне лишний new — что слону дробина.
Здравствуйте, elmal, Вы писали:
E>Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
это же не рокет саенс (а бизнес логика привариваемая к UI), там тысячу (условно) таких объектов класса в цикле создай — никто и не заметит разницы, а оптимизации ради оптимизаций — это уже маразм ..
хотя (но мы код конечно не видели) вероятно кодерок рукожоп который (если мы правильно тебя поняли) не умеет в статические классы и методы (не знаю как там с ними в C#), но если брать и напр., "другие ньюансы того же C/C++" — в них тоже не все шарят, или не до конца (взять тот же Boost засранный шаблонами (в исходном коде), в котором чёрт ногу сломит — а то и даже сами разработчики, потому в семье не без урода в каждой версии — не без ошибок)
но, и в целом умельцев красиво (и грамотно) писать код — "единицы" (условно) .. остальные — отъявленные рукожопы ..
Здравствуйте, elmal, Вы писали:
E>…Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый. E>Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Вопрос было бы правильнее всего адресовать автору кода: зачем он сделал так, а не как-то иначе. Возможно, он предвидел, что а) в реальной практике нужно будет поменять так и эдак б) при необходимости добавить тесты будет проще расширитьть этот класс в) да всё что угодно. ИМХО, оценивание тестового задания без беседы потом о том, как оно выполненно и почему скорее отвечаетнг а вопрос "очень ли оно плохо сделано", а чтобы разобраться с тем, на сколько оно сделано хорошо имеет смысл поговоритоь с автором и тогда вещи, которые вызывают недоумение при первом ревью могут проясниться.
Help will always be given at Hogwarts to those who ask for it.
Здравствуйте, vaa, Вы писали:
E>>>Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар.
S>>ИМХО, гораздо хуже сабжа. Сабж может быть оправдан архитиктурно (даже при performance penalty), а вот это оправдать ничем нельзя, это просто дисквалификация.
«архитиктурно» — ужас какой. Сорри.
vaa>возможно берут на поддержку такого же овна, тогда это тоже оправдано, правильный чел не поймет. (как в джокере: — что смешного? — вы все равно не поймете.)
Меня как-то брали. В плохом смысле этого слова (такое вот ощущение духовного изнасилования). Я не хотел, но не устоял перед уровнем оплаты. Честно скажу, понимать там особо нечего (ну, или я неправильный). Просто выносишь очередную тысячу строк, где интерпретируется скрипт, управляющий оборудованием, из OnCombo1Change() в отдельный класс и скрещиваешь пальцы, чтобы у тысяч клиентов после этого ничего не отпало. (Тестов-то тоже нет, да их и не написать толком). Ну и, конечно, прямые отрисовки заменяешь на колбэки/сообщения.
Здравствуйте, elmal, Вы писали:
E>Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
Такой стиль пропагандирует Мартин в своём Чистом Коде. Если у нас есть функция с n локальными переменными и 100500 строчек кода, то можно из неё сделать отдельный класс, локальные переменные сделать полями, а код декомпозировать по методам.
В языках, позволяющих определять функции внутри функций, этот подход устарел.
Здравствуйте, scf, Вы писали:
scf>Такой стиль пропагандирует Мартин в своём Чистом Коде. Если у нас есть функция с n локальными переменными и 100500 строчек кода, то можно из неё сделать отдельный класс, локальные переменные сделать полями, а код декомпозировать по методам. scf>В языках, позволяющих определять функции внутри функций, этот подход устарел.
Победа ООП над силами разума.
Здравствуйте, Kernan, Вы писали:
K>С чего бы это? ООП это же про внутреннее состояние. Если его не надо хранить, то смысла в объектах нет — достаточно просто написать функцию и всё. Для обрработки или вычиисления чего либо достаточно функции, а если её применить совместно с стрим апи каким-нибудь, то будет вообще всё красиво.
Потому что с некоторых пор в наши головы начали вбивать мысль (и довольно успешно, надо сказать), что просто написать код по имеющемуся ТЗ — этого недостаточно. Надо еще позаботиться о том, чтобы это могло рефакториться, чтобы тот, кто будет это изменять, мог легко и просто превратить эти брюки в элегантные шорты и т.д.
И тут автор начинает задумываться. А что , если со временем этот процесс калькуляции будет изменен ? Если в нем будет не простенькая формула, а несколько стадий, с передачей данных между ними ? Передача данных и static — это вещи почти несовместимые, в многопоточном приложении нарвешься на неприятности. Нет уж, лучше я сделаю класс и экземпляр, тут я твердо уверен, что то самое внутреннее состояние, о котором ты пишешь, мне легко сделать зависящим от него только и ни от чего больше. Добавляю я туда несколько полей, реализую логику вычислений по иному — никто, кроме этого экземпляра и знать не будет.
PD>>Увы, это стало почти что нормой и объяснить , что это плохо, очень трудно. K>Да нифига. Там где надо требуют понимание работы GC, синхронизации и disposal всяких.
Ключевое слово "Где надо". Ни из чего не следует, что тот, о ком пишет ТС, работал "там где надо"
Здравствуйте, Kernan, Вы писали:
K>С чего бы это? ООП это же про внутреннее состояние. Если его не надо хранить, то смысла в объектах нет — достаточно просто написать функцию и всё. Для обрработки или вычиисления чего либо достаточно функции, а если её применить совместно с стрим апи каким-нибудь, то будет вообще всё красиво.
В современном коде очень немного классов с мутабельным состоянием. 99% классов пишутся для отделения интерфейса от реализации и Dependency Injection. Прежде всего для того, чтобы иметь возможность добавлять зависимости в функцию и менять реализацию, не затрагивая вызывающий код.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>И тут автор начинает задумываться. А что , если со временем этот процесс калькуляции будет изменен ? Если в нем будет не простенькая формула, а несколько стадий, с передачей данных между ними ? Передача данных и static — это вещи почти несовместимые, в многопоточном приложении нарвешься на неприятности. Нет уж, лучше я сделаю класс и экземпляр, тут я твердо уверен, что то самое внутреннее состояние, о котором ты пишешь, мне легко сделать зависящим от него только и ни от чего больше. Добавляю я туда несколько полей, реализую логику вычислений по иному — никто, кроме этого экземпляра и знать не будет.
В таких случаях делают фабрику, или как минимум фабричный метод. Тогда убивается сразу 2 зайца: возможность замены реализации и возможность тестирования: замена всяких "calculate" при написании юнит-тестов — регулярная задача. Притом не важно как реализована фабрика или фабричный метод — важно что они есть и их можно переопределить.
new по месту, да ещё и в цикле — просто явный признак того, что человек никогда не занимался оптимизацией.
Будь там вот такое, то это был бы совсем другой разговор.
for (;;)
{
caclulator = GetCaclulator();
caclulator.Calculate(data);
}
Всё сказанное выше — личное мнение, если не указано обратное.
Я тут недавно поревьюил по сторонней просьбе некоторый код на C#,что было тестовым заданием в некоторую контору. Автор программит профессионально более 10 лет.
Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар. Тут ладно, придираюсь. Нет тестов вообще — ладно, тоже нормально (хотя почему то у меня даже на тестовое задание всегда требовали тесты). Все отправлено в виде одного коммита в GIT, и не видно истории как все делалось — допустим тоже так и надо. Ну и всякие GOD methods.
Но мне вот интересно, это я такой непродвинутый и это только меня напрягает?
Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Здравствуйте, elmal, Вы писали:
E>Я тут недавно поревьюил по сторонней просьбе некоторый код на C#,что было тестовым заданием в некоторую контору. Автор программит профессионально более 10 лет.
E>Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар. Тут ладно, придираюсь. Нет тестов вообще — ладно, тоже нормально (хотя почему то у меня даже на тестовое задание всегда требовали тесты). Все отправлено в виде одного коммита в GIT, и не видно истории как все делалось — допустим тоже так и надо. Ну и всякие GOD methods.
E>Но мне вот интересно, это я такой непродвинутый и это только меня напрягает?
E>Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
E>Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
придираетесь))
жестянщик не обязан уметь строить ракеты как ракетчик, хотя оба работают с материалами.
скорость проф роста также зависит от способностей, возможностей, приоритетов.
Здравствуйте, elmal, Вы писали:
E>Я тут недавно поревьюил по сторонней просьбе некоторый код на C#,что было тестовым заданием в некоторую контору. Автор программит профессионально более 10 лет.
А на чём программит последние 3-5?
E>Но мне вот интересно, это я такой непродвинутый и это только меня напрягает?
Кроме гита, всё правильно говоришь. Народ совсем разучился или не умел писать на ОО языках в ОО парадигдме. Нарушение SRP, GOD классы, неумение сделать объектную декомпозицию хоть как-то. Просто берут и фигачат код. Такое ощущение, что никто не читал книг по ООП вообще, ни книг по архитектуре и не применяли это на практике.
E>Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
Это коммент как минимум в код ревью средней сиверити, т.е. new в цикле даже на C# это не особо хорошо.
E>Я был только сторонний наблюдатель и на это посмотрел после того, как это было отправлено, просто как сторонний консультант по просьбе автора. Непосредственно тех, кто оценивал тестовое задание — все удовлетворило. Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
А разве static методов C# нет? Взяли его на работу в итоге?
Здравствуйте, scf, Вы писали:
scf>Такой стиль пропагандирует Мартин в своём Чистом Коде. Если у нас есть функция с n локальными переменными и 100500 строчек кода, то можно из неё сделать отдельный класс, локальные переменные сделать полями, а код декомпозировать по методам. scf>В языках, позволяющих определять функции внутри функций, этот подход устарел.
Там нет n локальных переменных. Там ровно один параметр на входе и один на выходе, после чего никакое внутреннее состояние объекта не интересно. По существу там чистая функция, и что то я сомневаюсь что Мартин бы такое посоветовал. static метод определенного класса полностью бы решил все проблемы (пусть даже и с созданием экземпляра внутри, ибо там Thread Safe требуется).
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Увы, это стало почти что нормой и объяснить , что это плохо, очень трудно.
Тут проблема то ИМХО не в быстродействии, а в раздувании количества строчек и кода на ровном месте.
То есть вместо одной сторочки:
var result = Calculator.Сalculate(data)
или даже
var result = Сalculate(data)
, на ровном месте появляется две:
var calculator = new Calculator();
var result = calculator.calculate(data);
Здравствуйте, elmal, Вы писали:
E>Здравствуйте, Pavel Dvorkin, Вы писали:
PD>>Увы, это стало почти что нормой и объяснить , что это плохо, очень трудно. E>Тут проблема то ИМХО не в быстродействии, а в раздувании количества строчек и кода на ровном месте.
E>То есть вместо одной сторочки:
E>
E>var result = Calculator.Сalculate(data)
E>
E> или даже E>
E>var result = Сalculate(data)
E>
E>, на ровном месте появляется две:
E>
E>var calculator = new Calculator();
E>var result = calculator.calculate(data);
E>
E>И ведь не лень ...
ну будут 2 строчки не в одном месте, а в разных, что изменится. реальная проблема здесь только в лишнем выделении памяти в цикле.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Идеологически — создание нового экземпляра класса верно и оправданно. Если мне нужна новая табуретка, то надо сделать новую табуретку, а не брать табуретку, которую я сделал год назад и пытаться ее переделать под требования к новой табуретке.
Однако как создавать? Если брать табуретку, то идеологически наверно у нас есть фабрика табуреток, которой мы скармливаем необходимые материалы и говорим сделай табуретку. И фабрика табуреток у нас одна и нам не нужно для каждой табуретки сначала стрить фабрику. Здесь же самое близкое — это мы создаем объект табуретка и у этого объекта дергаем метод создайся. А на деле там еще веселее. Нужно, условно говоря, посчитать значение какой формулы. На входе выражение, на выходе число. Логически с точки зрения использования одна функция, причем чистая функция. Но практически создают новый экземпляр класса интерпретатора выражений с одним методом "вычислить". Хорошо хоть не додумались этому интерпретатору выражений сначала посылать сообщение "получи входные данные", потом сообщение "посчитай", а потом посылать сообщение отдай "результат". А то ведь и до такого можно было додуматься в теории.
Я просто несколько удивился, что можно проработать в индустрии более 10 лет, на разных работах, на буржуйских заказчиков, и при этом за подобное не получать по рукам и не делать по умолчанию наиболее удобно к использованию.
Здравствуйте, Буравчик, Вы писали:
Б>Минусы статиков по сравнению с обычным классом: Б>- если логика сложная — то все вспомогательные функции тоже должны быть статик
Никаких проблем, это вообще не минус.
Б>- если появятся параметры класса, то придется все статики переделать в нормальные методы (а параметры обязательно появятся)
С чего им появляться? Это класс — функция. В теории там если что и появится, то конфиг. Конфиг можно передать вторым аргументом при выполнении функции или параметрами по умолчанию.
Б>- код прибит к классу — никакого тестирования, никакого IoC
Тестирование элементарное — это чистая функция, один вход, один выход. Никаких сайдэффектов вообще, тестирование идеальное и образцовое, даже сторонних зависимостей и ввода вывода нет.
Б>P.S. Ну и в целом, класс с одним публичным методом — нормальная практика при использовании IoC и DI
Да, нормальная. Но IoС и DI не использовались.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Ну этот вопрос спорный. Как городской житель я, конечно, закажу табуретку фабрике. Но сельский житель вполне возможно сделает ее сам. В прошлом году жена пристала — сделай табуретку. Сделал ей, квадратную. Так она в каком-то журнале увидела круглую табуретку и теперь требует такую. Квадратная ей больше не нужна. Пытаюсь сделать из квадратной круглую — что-то плохо получается .А фабрики в деревне нет.
Под фабрикой я подразумеваю не паттерн фабрика, а тот, кто создает определенные экземпляры. Табуретка не будет создавать сама себя, табуретку создает что то (станок), или кто то (конкретный человек или даже коллектив). Соответствующие методы создания будут уже относиться к конкретным другим классам, причем зачастую через интерфейс CanCreateStool с одним методом. При этом сам класс табуретка имплементить такое не может ибо это будет концептуальная ошибка. Это если действительно заморачиваться с истинным ООП, лично я крайне ленив и предпочитаю писать минимум кода и все эти фабрики я пишу только при крайней необходимости, когда от их наличия кода оказывается меньше, а не как блин некоторые, пихают эти фабрики везде чтоб показать как они круто ООП знают и понимают, и раздувают код на ровном месте в разы.
PD>Ну вообще-то небольшой нюанс тут есть. На static можно нехило проколоться в многопоточном приложении. Добавишь статическое поле — и привет.
Можно, но нечего использовать по существу глобальные переменные без крайней необходимости. static предполагает что никаких статических полей использоваться не будет, только константы.
PD>Где-то такое приведет к приличной просадке скорости или проблемам с памятью. А где-то ничего заметного не будет. Например, в GUI — пока приложение вызовет ОС для отрисовки, а ОС перебросит все это видеодрайверу, а тот будет думать, как это лучше отрисовать, потому что у него таких приложений много, а потом начнет шрифты рендерить.. На этом фоне лишний new — что слону дробина.
Я могу ошибаться, то именно относительно производительности сам копилятор и оптимизатор в теории может этот new и не делать. А даже если и сделает, то это будет первое поколение объектов накладные расходы на сборку мусора и создание довольно ничтожны. А вот то, что вызывающий код будет вынужден создавать объект и затем у него вызывать метод — это уже более неприятно, ибо это заставляет пользователя писать больше кода на ровном месте.
Здравствуйте, Quebecois, Вы писали:
Q>Если Calculate() хоть сколько-либо нетривиально, лишнее выделение памяти здесь погоды не сделает.
out string выглядит как кандидат на логирование или на что еще, с выводом промежуточных результатов куда либо. Лично я для этого использую подход шины сообщений. В случае, если я живу без DI IOC фреймворков, у меня есть глобальный доступный отовсюду объект eventBus, с двумя методами — publish и subscribe (ладно, трeмя — unsubscribe может еще потребоваться). Соответственно там, где нужно уведомить внешний код, я в расчетной части буду кидать соответствующие события. Ну а если я уже живу с фреймворками, то функционал шины сообщений там как правило уже встроен, ну и у меня уже никаких статиков — все классы конструируются и инжектятся зависимости уже фреймворком, соответственно у меня вообще никаких new в коде практически нет.
Собственно с точки зрения внешнего кода подход с классом все равно чуть более громоздкий — один хрен нужно передавать параметры, теперь м их передаем в конструкторе и можем еще дополнительно просетать всякие второстепенные параметры отдельно. Преимуществ подобного подхода особо не вижу. А если мы останавливаемся на статике и не хотим в промежуточные функции передавать до черта аргументов — мы внутри статического метода конструируем объект (точнее даже структуру зачастую, ибо методы не нужны) контекста выполнения и уже контекст выполнения передаем на подрасчеты. Но при этом вся сложность и все проблемы самого расчета клиента вызывающего кода не касаются никак, для вызывающего кода это чистая функция без сайдэффектов.
Есть конечно еще один подход — параметры передавать через билдеры, возвращая ссылку на сам билдер чтобы можно было в цепочку инициализации выстраивать. Но такое ИМХО нужно использовать далеко не по умолчанию и 100 раз подумать, нужно таким заморачиваться или нет смысл усложнять клиентский код, лично я такое считаю оправданным только в какой тяжелой библиотеке на этапе инициализации один раз во всем приложении.
Здравствуйте, elmal, Вы писали:
E>Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар.
ИМХО, гораздо хуже сабжа. Сабж может быть оправдан архитЕктурно (даже при performance penalty), а вот это оправдать ничем нельзя, это просто дисквалификация.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Ну этот вопрос спорный. Как городской житель я, конечно, закажу табуретку фабрике. Но сельский житель вполне возможно сделает ее сам. В прошлом году жена пристала — сделай табуретку. Сделал ей, квадратную. Так она в каком-то журнале увидела круглую табуретку и теперь требует такую. Квадратная ей больше не нужна. Пытаюсь сделать из квадратной круглую — что-то плохо получается .А фабрики в деревне нет.
Здравствуйте, Shtole, Вы писали:
S>Здравствуйте, elmal, Вы писали:
E>>Так вот, я молчу про то, что в бизнес логике вызываются напрямую всякие методы UI отрисовки вида увеличить прогресс бар.
S>ИМХО, гораздо хуже сабжа. Сабж может быть оправдан архитиктурно (даже при performance penalty), а вот это оправдать ничем нельзя, это просто дисквалификация.
возможно берут на поддержку такого же овна, тогда это тоже оправдано, правильный чел не поймет. (как в джокере: — что смешного? — вы все равно не поймете.)
I used to love utility classes filled up with static methods. They made a great consolidation of helper methods that would otherwise lie around causing redundancy and maintenance hell. They're very easy to use, no instantiation, no disposal, just fire'n'forget. I guess this was my first unwitting attempt at creating a service oriented architecture — lots of stateless services that just did their job and nothing else. As a system grows however, dragons be coming.
Polymorphism
Say we have the method UtilityClass.SomeMethod that happily buzzes along. Suddenly we need to change the functionality slightly. Most of the functionality is the same, but we have to change a couple of parts nonetheless. Had it not been a static method, we could make a derivate class and change the method contents as needed. As it's a static method, we can't. Sure, if we just need to add functionality either before or after the old method, we can create a new class and call the old one inside of it — but that's just gross.
Interface woes
Static methods cannot be defined through interfaces for logic reasons. And since we can't override static methods, static classes are useless when we need to pass them around by their interface. This renders us unable to use static classes as part of a strategy pattern. We might patch some issues up by passing delegates instead of interfaces.
Testing
This basically goes hand in hand with the interface woes mentioned above. As our ability of interchanging implementations is very limited, we'll also have trouble replacing production code with test code. Again, we can wrap them up but it'll require us to change large parts of our code just to be able to accept wrappers instead of the actual objects.
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.
Parameter creep
To begin with, that little cute and innocent static method might take a single parameter. As functionality grows, a couple of new parameters are added. Soon further parameters are added that are optional, so we create overloads of the method (or just add default values, in languages that support them). Before long, we have a method that takes 10 parameters. Only the first three are really required, parameters 4-7 are optional. But if parameter 6 is specified, 7-9 are required to be filled in as well... 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. 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
One of the most common arguments is, why demand that consumers of our class create an instance for invoking this single method, while having no use for the instance afterwards? Creating an instance of a class is a very very cheap operation in most languages, so speed is not an issue. Adding an extra line of code to the consumer is a low cost for laying the foundation of a much more maintainable solution in the future. And finally, if you want to avoid creating instances, simply create a singleton wrapper of your class that allows for easy reuse — although this does make the requirement that your class is stateless. If it's not stateless, you can still create static wrapper methods that handle everything, while still giving you all the benefits in the long run. Finally, 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
Of course, there are exceptions to my dislike of static methods. True utility classes that do not pose any risk to bloat are excellent cases for static methods — System.Convert as an example. 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.
Standards, standards, standards!
Using instance methods does not inhibit you from also using static methods, and vice versa. As long as there's reasoning behind the differentiation and it's standardised. There's nothing worse than looking over a business layer sprawling with different implementation methods.
Only a Sith deals in absolutes
Of course, there are exceptions to my dislike of static methods. True utility classes that do not pose any risk to bloat are excellent cases for static methods — System.Convert as an example. 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.
Тестовое задание, это как раз такой случай — система никогда не будет расти,более того, там как раз утилитный класс вроде System.Convert.
Во вторых, как в этом топике другие сказали — есть принципы KISS и YAGNI.
И в третьих. Когда "As a system grows however, dragons be coming." Когда система растет, скорее всего будут использоваться DI IOC фреймворки и уже тема для холивара не становится актуальной, мы сами никакой оператор new не используем, за нас это делает фреймворк и он же занимается начальной инициализацией. Более того, на начальном этапе скорее всего будут использоваться фреймворки.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Идеологически — создание нового экземпляра класса верно и оправданно.
С чего бы это? ООП это же про внутреннее состояние. Если его не надо хранить, то смысла в объектах нет — достаточно просто написать функцию и всё. Для обрработки или вычиисления чего либо достаточно функции, а если её применить совместно с стрим апи каким-нибудь, то будет вообще всё красиво.
PD>Если мне нужна новая табуретка, то надо сделать новую табуретку, а не брать табуретку, которую я сделал год назад и пытаться ее переделать под требования к новой табуретке.
Только тебе не табуретка нужна, а сеть где-то.
PD>Увы, это стало почти что нормой и объяснить , что это плохо, очень трудно.
Да нифига. Там где надо требуют понимание работы GC, синхронизации и disposal всяких.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Потому что с некоторых пор в наши головы начали вбивать мысль (и довольно успешно, надо сказать), что просто написать код по имеющемуся ТЗ — этого недостаточно. Надо еще позаботиться о том, чтобы это могло рефакториться, чтобы тот, кто будет это изменять, мог легко и просто превратить эти брюки в элегантные шорты и т.д.
Что-то мне кажется что ты давно не работал над масштабнымии вещами.
PD>И тут автор начинает задумываться. А что , если со временем этот процесс калькуляции будет изменен ? Если в нем будет не простенькая формула, а несколько стадий, с передачей данных между ними ?
И что? Несколько стадий неплохо разбиваются на несколько функций.
PD>Передача данных и static — это вещи почти несовместимые, в многопоточном приложении нарвешься на неприятности. Нет уж, лучше я сделаю класс и экземпляр, тут я твердо уверен, что то самое внутреннее состояние, о котором ты пишешь, мне легко сделать зависящим от него только и ни от чего больше. Добавляю я туда несколько полей, реализую логику вычислений по иному — никто, кроме этого экземпляра и знать не будет.
Чего? Я говорил про статическую ФУНКЦИЮ в классе которая только считает т.к. в шарпах нет просто функций, класс без полей если что. Такое многопоточится на раз если функция не даёт "сайд эффектов" или как там это называется. А ты добавил полей, а кто-то заюзал один экземпляр твоего калькулятора в нескольких потоках и у него всё заработало, а вот в проде всё не заработало потому что дата рейсы никуда не пропали.
Здравствуйте, Kernan, Вы писали:
PD>>Потому что с некоторых пор в наши головы начали вбивать мысль (и довольно успешно, надо сказать), что просто написать код по имеющемуся ТЗ — этого недостаточно. Надо еще позаботиться о том, чтобы это могло рефакториться, чтобы тот, кто будет это изменять, мог легко и просто превратить эти брюки в элегантные шорты и т.д. K>Что-то мне кажется что ты давно не работал над масштабнымии вещами.
Верно кажется. Но то, о чемя написал, существует уже давно. Вот когда я начинал, этим точно не заморачивались.
PD>>И тут автор начинает задумываться. А что , если со временем этот процесс калькуляции будет изменен ? Если в нем будет не простенькая формула, а несколько стадий, с передачей данных между ними ? K>И что? Несколько стадий неплохо разбиваются на несколько функций.
См. ниже.
PD>>Передача данных и static — это вещи почти несовместимые, в многопоточном приложении нарвешься на неприятности. Нет уж, лучше я сделаю класс и экземпляр, тут я твердо уверен, что то самое внутреннее состояние, о котором ты пишешь, мне легко сделать зависящим от него только и ни от чего больше. Добавляю я туда несколько полей, реализую логику вычислений по иному — никто, кроме этого экземпляра и знать не будет. K>Чего? Я говорил про статическую ФУНКЦИЮ в классе которая только считает т.к. в шарпах нет просто функций, класс без полей если что. Такое многопоточится на раз если функция не даёт "сайд эффектов" или как там это называется.
Я же написал — а если потом появятся поля для передачи промежуточных данных ? Я же написал.
>А ты добавил полей, а кто-то заюзал один экземпляр твоего калькулятора в нескольких потоках и у него всё заработало, а вот в проде всё не заработало потому что дата рейсы никуда не пропали.
Откуда тут дата рейсы ? В каждом потоке свой экземпляр этого калькулятора, общих данных нет. А вот со static не получится.
Здравствуйте, Pavel Dvorkin, Вы писали:
PD>Все зависит от того, чем именно он занимался. PD>Где-то такое приведет к приличной просадке скорости или проблемам с памятью. А где-то ничего заметного не будет. Например, в GUI — пока приложение вызовет ОС для отрисовки, а ОС перебросит все это видеодрайверу, а тот будет думать, как это лучше отрисовать, потому что у него таких приложений много, а потом начнет шрифты рендерить.. На этом фоне лишний new — что слону дробина.
Я бы так не сказал, учитывая что после видеодрайвера работу начнет соотв. железо, т.е. это будет быстро и по памяти не очень много сожрет ( хотя зависит от).
А вот множество new и по времени и по памяти да еще + GC могут дорого обойтись.
Здравствуйте, sergii.p, Вы писали:
SP>Здравствуйте, elmal, Вы писали:
E>>Короче, важно быстродействие, кое что выполняется в цикле. По смыслу — выполнение функции, один вход, один выход. Однако автор создает экземпляр определенного класса через new без параметров и далее выполняет функцию calculate, тоже без параметров. После чего созданный объект забывается и в цикле создается новый.
SP>мне кажется, тут ноги растут из просчёта языка, что нельзя создать свободную функцию. Вот и приходится добавлять "классы ради классов" с функциональностью областей видимости (namespace). А то как оно вызывается — абсолютно незначимый момент. SP>Я даже не понимаю зачем это ограничение до сих пор в стандарте. Можно было уже сотню раз раскаяться и не плодить костыли вроде static class.
SP>В общем, какой язык, такие и программисты
Полностью согласен стиль программирования в ЯП зависит от культуры сообщества.
Локальные функции добавили, но ввиду мутабельности и null по умолчанию в шарпе они не очень удобны,
да еще могут порождать трудноуловимые мутации внешних к фукции переменных.
вообщем фигурные скобки это диагноз. но яп не худший, если учесть отличную ВМ и базовые библиотеки типа линка.
больше всего удручает его многословность при создании массивов и словарей например и определение типа перед именем. было бы лучше
Здравствуйте, scf, Вы писали:
scf>В современном коде очень немного классов с мутабельным состоянием.
Наверное, в большинстве современных проектов где основные задачи это преобразование данных из сетевой трубы в трубу БД, а не управление состояниями. Я, видимо, в своём пузыре с управлением состояниями сидел последние лет 10. scf>99% классов пишутся для отделения интерфейса от реализации и Dependency Injection. Прежде всего для того, чтобы иметь возможность добавлять зависимости в функцию и менять реализацию, не затрагивая вызывающий код.
Но ведь этого можно добиться функциями высшего порядка. Ладно, в любом случае это всё сорта одного и того же.
Здравствуйте, sergii.p, Вы писали:
SP>мне кажется, тут ноги растут из просчёта языка, что нельзя создать свободную функцию. Вот и приходится добавлять "классы ради классов" с функциональностью областей видимости (namespace). А то как оно вызывается — абсолютно незначимый момент.
Большой разницы между статическими методами и настоящими функциями нет.
Модуль с функциями = класс со статическими методами.
Вышеперечисленные проблемы присутствуют у обоих (в отличии от обычного класса).
E>var calculator = new Calculator();
E>var result = calculator.calculate(data);
E>
E>И ведь не лень ...
Надо смотреть весь код. Возможно, Calulator — это struct, и там никакого "выделения памяти" не происходит, а происходит обнуление состояния этого калькулятора.
Возможно, там вызов calculate проинлайнился, и вообще вся инициализация выбросилась JIT-ом вместе с экземпляром, и даже регистр этой переменной не занят.
Возможно, этот код написан внутри замыкания, и calculator был залифтен в поле класса, которое устраняется джитом только в том случае, если инициализация проведена прямо здесь, чтобы инлайнер её видел.
Уйдемте отсюда, Румата! У вас слишком богатые погреба.
Здравствуйте, maxkar, Вы писали:
M>Тоже связано с тем, что с абстракциями не очень (правда уже применительно к процессу разработки). Не факт, что там вообще было промежуточное работающее остояние, которое можно было бы закоммитить. А может быть он из среды, где все коммиты сливаются в один при мерже, так зачем стараться?
Ну вот хз, лично я коммичу часто. Даже в самом начале проекта. Основной критерий — чтоб компитировалось, что не работает и до хрена TODO пофиг. Да и не на начальном этапе тоже, по умолчанию использую trunk based development с feature flags и вполне никому не мешаю и редко что ломаю. Хотя я в курсе что мало кто так делает, про git flow я знаю. При желании могу и красиво и правильно делать, как положено, что прям историю почитаешь и душа радуется. Но без необходимости заморачиваться нет желания.
M>Все вышенаписанное вообще должно делаться автоматически. Это стандартные проблемы дизайна со стандартными решениями и причинами таких решений. Там даже мозг можно не включать!
Именно! Я в свое время даже давал тестовое задание с ограничением по времени (час) на проверку рефлексов. Народ хреначил сутки и выдавал такую мешанину фабрик фабрик и применял все паттерны проектирования, при этом не расширяемо ни как. Через определенное время возникли сомнения и я прям параллельно с кандидатом с 23 часа ночи решил сам сделать свое тестовое задание. Уложился за 30 минут, не смотря что уставший был в усмерть, хотя там работы вообще на 10 минут максимум. Потом сам свое эталонное решение показываю кандидату — прям откровение было. Откровение что можно оказывается не городить классы и фабрики, а тупо на статических методах быстро сделать поддерживаемое расширяемое решение с нужными уровнями абстракции, которые крайне легко заменять.
И это не означает что я всегда только пишу на статических методах, иногда я вполне использую самое настоящее ООП, прям эталонное. То когда требуется, я не ООП ради ООП.
Здравствуйте, Sharov, Вы писали:
S>Я бы так не сказал, учитывая что после видеодрайвера работу начнет соотв. железо, т.е. это будет быстро и по памяти не очень много сожрет ( хотя зависит от). S>А вот множество new и по времени и по памяти да еще + GC могут дорого обойтись.
Я имел в виду один или несколько лишних new на фоне рисования GUI, в каком-нибудь OnPaint. А так-то да, конечно.
Здравствуйте, maxkar, Вы писали:
M>А может быть, реализация имеет высокую связность с классом пользователя и мы ее сделаем в том же классе реализации. Декораторами интерфейса можно добавить всякие красивости в виде логирования и метрик (и их можно потом использовать с другими реализациями логики калькулятора).
А можешь написать пример с декоратором если не сложно?
Здравствуйте, 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 — там это так не работает. С последними версиями шарпа не работал.
Всё сказанное выше — личное мнение, если не указано обратное.
Здравствуйте, elmal, Вы писали:
E>... Отсюда вопрос, я что — действительно слишком придираюсь, или по крайней мере в C# является бест практикой на каждый чих создавать класс с одним публичным методом и чтоб вызывающий код для простейшего случая не вызывал одну функцию, а вынужден сначала создавать объект оператором new, а затем вызывать один единственный метод чтоб что то посчитать?
Это ж тестовое задание. Глупость само по себе — проверяет только то, как человек сделает написанное на бумажке задание за минимум времени. Неизвестно, как автор будет писать в рамках кодовой базы вашей конторы. Может, адаптируется и всё будет норм, а может, уволите через пару месяцев. Все это еще и сильно зависит от того, насколько в вашей конторе ценится код. Судя по тому, что ты пишешь о "практиках" — очень ценится, но это совсем не везде так.