How to become an author
Search
Write a publication
Pull to refresh

Comments 48

UFO just landed and posted this here
Не совсем понятно что конкретно Вы здесь собираетесь тестировать. В конечном счете wcf сервис это обычный код на сервере, и тестироваться он может также как и любой другой код.
Total votes 3: ↑3 and ↓0+3
За ребус спасибо. Действительно интересно выглядит.
Total votes 2: ↑1 and ↓10
А почему бы не следовать стандартным паттернам и не сделать Do(), BeginDo(...), EndDo(...)?
Ваше решение порождает кучу проблем с тем, что вызывающий код не знает, выполняется ли Do синхронно или асинхронно (а теперь на секунду представим, что там, внутри, вылетел exception). Одно предложение «возможно через месяц их придется сделать опять синхронными» подтверждает мои слова.
И не стоит говорить что ваш void-возвращающий метод Pure, ибо такого небывает (или такой метод бесполезен).
В любом случае вам пришлось модифицировать тесты, как минимум их инициализаторы.
Total votes 7: ↑7 and ↓0+7
Еще раз подчеркну, что код вида:
{
    Task.Factory.StartNew(...);
}
Является преступлением, т.к. образуется висячий объект Task, на который у вас нет ссылок, и если внутри таска возникнет исключение — оно выбросится в момент отработки финализатора для объекта Task, т.е. недетерминированно, а это совсем-совсем не хорошо.

Рассчитывать на то, что в знакомом вам коде не будет исключений — это слишком самонадеянно (особенно в .net, где есть те-же TypeInitializationException), а советовать такой подход другим людям — просто невежественно.
Total votes 10: ↑9 and ↓1+8
Внутри таска у нас ВСЕ исключения обрабатываются, поэтому я могу быть ПОЛНОСТЬЮ уверен что оно не вылетит наружу. А невежественно использовать чужой код не понимая как он работает и какие проблемы в себе несет. В конце топика я специально сделал на этом акцент.
Total votes 3: ↑2 and ↓1+1
Я так понимаю там catch(Exception ex)? Или может вообще просто catch? Так нельзя делать. Или вы заранее уверены во всех возможных исключениях? И интересно, как же вы собираетесь вызывающему коду сообщить о проблеме из висящего Task'а?

В итоге получается быдлокод на быдлокоде, все эксепшены мы проглотим, а на стабильность работы наплюем. Ну удачи.
Total votes 9: ↑8 and ↓1+7
Мне кажется бессмысленно спорить о качестве абстрактных коней в вакууме. Все понимают что молча глотать все исключения плохо. Все понимают что приложение должно быть стабильным и сообщать о возможных проблемах. Но что мне всегда не нравилось, так это делать работу ради того чтобы сделать работу. Рефакторить код ради того чтобы отрефакторить код. Сообщать о проблемах в висящем таске потому что правильно сообщать о проблемах в висящем таске. Если этот таск присылает мне свежие анекдоты каждый вечер, то мне не нужно с утра лезть в лог сервера, чтобы понять что вчера вечером таск не сработал и не прислал мне свежие анекдоты из-за того что лежал мыльный сервер. Я и так узнаю что таск не сработал, т.к. не получу долгожданный анекдот. И не расстроюсь. Т.к. знаю что этот сервер периодически лежит, и скорее всего завтра анекдот придет снова. И в таком случае я не буду реализовывать уведомления по смс/телефону/радио и т.д., потому что в ДАННОМ КОНКРЕТНОМ случае это не нужно. Затраты не оправдывают результат. Если уж Вы волнуетесь за тех кто скопирует приведенный мной для примера код и завтра из-за этого случайно вылетит баллистическая ракета, то боюсь что она вылетит и без моего топика, возможно неделей позже :)
Total votes 13: ↑5 and ↓8-3
Мне кажется абсолютно бессмысленным оправдание быдлокода, и, тем более, статей, предлагающих использование этого быдлокода, только тем, что по мнению автора задача не стоит выеденного яйца и к ней можно относиться с любой степенью халатности.

Давайте еще предлагать делать скрутки из 10 обрезков витухи, замотанные скотчем, только потому, что сеть никому не нужна и делается для галочки, давайте агитировать ставить zverdvd, потому что компом в вашем конкретном случае все равно не будут пользоваться. Да нет проблем, но тогда делайте это тихо, и никому не рассказывайте, и, тем более, не рекомендуйте.
Total votes 7: ↑6 and ↓1+5
Я новичок на хабре, а опыт он приходит с годами. Не думал что каждый байт демонстрируемого здесь кода воспринимается читателями как рекомендация к его использованию в неизменном виде. Мне казалось что каждый его профильтрует и возьмет для себя то что посчитает нужным. В следующий раз буду более осмотрителен при выборе примеров.
Total votes 3: ↑3 and ↓0+3
Хабр не при чем, в любом месте в любом описании давать заведомо вредные примеры — моветон.
Total votes 2: ↑2 and ↓0+2
> А почему бы не следовать стандартным паттернам и не сделать Do(), BeginDo(...), EndDo(...)?

Поделитесь о каких паттернах Вы говорите. Когда я пишу код, я люблю чтобы он рассказывал мне и другим что он делает.

> Ваше решение порождает кучу проблем с тем, что вызывающий код не знает, выполняется ли Do синхронно или асинхронно (а теперь на секунду представим, что там, внутри, вылетел exception).

Вообще то я об этом написал в топике

> И не стоит говорить что ваш void-возвращающий метод Pure, ибо такого небывает (или такой метод бесполезен).

Можете не верить, но так и есть

>В любом случае вам пришлось модифицировать тесты, как минимум их инициализаторы.

Тесты не трогали.

Total votes 2: ↑0 and ↓2-2
Поделитесь о каких паттернах Вы говорите.
CLR APM и Task'и, в зависимости от того, что там асинхронно делается.
Вообще то я об этом написал в топике
Где? В последнем абзаце? Т.е. дать заведомо бажное и неприменимое решение и сказать «осторожнее с проблемами», не описав проблем и не указав, что в приведенном решении их нельзя обойти?
Можете не верить, но так и есть
Pure метод, возвращающий void ничего не делает, если он меняет состояние на сервере или что-то делает с общими ресурсами системы — он уже ни разу не Pure. Иначе зачем его вызывать, если приложению фиолетово, завершится он удачно или нет?
This comment wasn’t rated yet0
А, значит я Вас не понял, мне показалось Вы говорите о наименовании методов. Спасибо за ссылочки. Об асинхронности можно отдельную статью писать. Я хотел написать о подходе к модификации поведения и простейшей реализации «стратегии». Даже стало интересно сколько человек скопирует код «как есть» к себе в проект, закрыв глаза на ремарку с указанными проблемами.
This comment wasn’t rated yet0
CLR APM включает в том числе и рекомендации по именованию методов.
Стратегия — хорошо, пример, на котором она показана — плохо, очень плохо.
Если он даже хоть как нибудь применим в вашей конкретной маленькой задаче маленького проекта — это не значит, что вам нужно о нем писать «в общем», преподнося как что-то, подходящее под более широкий класс случаев.

Если вы считаете что такой подход применим для реализации чего-то в сколько нибудь большом проекте — вы не правы. Если в проекте все места, которые по мнению их разработчиков слишком просты, будут сделаны абы как, то баги будет искать в разы труднее. Представьте, что вы вбежали в комнату, в которой 20см слоев выложен мусор и порезали ногу. Сколько времени нужно будет, чтобы найти стекло? Так же и в коде, если в куче мест все сделано кое-как, то найти, из за чего возник глюк становится сложнее, и более того — само это «абы как», несмотря на то, что оно по мнению авторов «слишком просто для того, чтобы делать качественно» будет давать собственные глюки.

А по поводу ремарки — обычно такую ремарку делают для методов, которые в чем-то превосходят остальные решения, и если эти проблемы можно как-то обойти или нейтрализовать и т.д.
В вашем же случае проблемы возникают неизбежно, и сам метод просто по всем параметрам проигрывает общепринятым подходам.
Total votes 1: ↑1 and ↓0+1
private Action doWork = work => { work(); };
Можно убрать вокруг work() фигурные скобки. Будет лямбда.

А вообще удивительно слышать что опытных разработчиков такая фундаментальная вещь как делегаты смущает. Или вы говорите о не-C# разработчиках?
Total votes 11: ↑6 and ↓5+1
Боже, что я несу, конечно же будет expression lambda. Сейчас это обычная statement lambda(Хотя безусловно в этом примере разницы никакой).
Total votes 4: ↑4 and ↓0+4
Да нет, делегаты их конечно не смущают, просто в голове приведенный в самом начале статьи кусок кода очень сложно компилируется :)
Total votes 2: ↑2 and ↓0+2
Пять минусов, ну и в карму полетело.

За что ?!

За то что я написал labmda вместо expression lambda? Вроде поправился ниже.

Или вам серьёзно кажется что это нормально для C# разработчика не понять выражение:
Action action = (Action action) => { action(); };

Все знают что в начале идёт тип, потом название переменной, дальше по => очевидно что это лямбда, а у лямбды сперва идёт объявление параметров а после знака => идёт тело, в данном случае вызов метода action() который разумеется является параметром этой лямбды, очевидно же что не может тело лямбды видеть переменную которой она присваивается в случае объединённой инициализации\объявления.

Меня _серьёзно_ смутило заявление о сложности\хитрости этого кода. Он абсолютно однозначен.

Возможно такой код ещё вызовет вопросы касательно результатов исполнение, не не простая же лямбда

List actions = new List(1000);
for (int i = 0; i < 1000; i++)
{
actions.Add(() => Console.WriteLine(i));
}

foreach (var action in actions)
{
action();
}
Total votes 6: ↑5 and ↓1+4
Да, и какой же его результат выполнения? Только с ходу!
This comment wasn’t rated yet0
1000 раз выведет 1000, а к чему (и даже больше — к кому) вопрос? Я сомневаюсь, что автор камента сам не знает ответа, раз уж привел такой пример.
This comment wasn’t rated yet0
Я тут обсуждал с товарищем замыкания, стыдно было бы начинать дискуссию не понимая что такое замыкания.

Да и вообще, замыкание на счетчик цикла — это ведь классика. Мне кажется уже все знают почему это происходит, и как этого избежать :)
This comment wasn’t rated yet0
А почему пошли путём внедрения поддержки стратегии внутрь класса, а не сделали многопоточный декоратор поверх класса?
И почему бы тогда не сделать классическую реализацию стратегии? С конечным множеством допустимых стратегий. Лямбда — это, конечно, изящно, но:
1) теоретически приводит к дублированию, ведь при каждом асинхронном использовании воркера нам нужно устанавливать одну и ту же лямбду стратегии.
2) Поймёт ли разработчик, который придёт через год, что вы имели в виду под этим Action?
3) Не возникнут ли подводные камни при определенных лямбдах? Ведь при таком подходе множество лямбд бесконечно, и вы не сможете проверить работу этого класса со всеми лямбдами.
4) Не внесет ли эта дополнительная свобода хаос, когда в одном месте Task.Factory, а в двух других ThreadPool?
5) Нужна ли эта свобода, вы планируете, что количество стратегий будет расти?
Total votes 1: ↑1 and ↓0+1
И да, по поводу исключений. Код, на который вы делегируете ответственность за вызов работы, ничего не должен знать о том, что исключения внутри DoWorkInternal обрабатываются. Иначе настанет день, когда кто-то решит, что глотать исключения — это bad practice, и уберёт там try/catch, не зная о том, что где-то есть лямбда с Task.Factory, тем самым нечаянно создав опасную ситуации.
This comment wasn’t rated yet0
Причем, как я понимаю, писать тесты на работу этого воркера в отдельном потоке, вы не планируете.
This comment wasn’t rated yet0
У этой задачи много решений, декоратор — одно из них. Плюс приведенного решения — отсутствие необходимости создавать дополнительные сущности.

Если предположить что стратегий будет только две, я бы все-равно не стал заводить дополнительные классы стратегий, а добавил бы конструктор в MyWorker с bool или enum параметром и в зависимости от него создавал тот или иной делегат внутри класса. И, честно говоря, существующего кода под рукой нет, не исключаю что там так и написано.

При определенных лямбдах проблеммы определенно возникнут, например при таком:

 (Action work) => { throw new Exception(); }
Total votes 1: ↑1 and ↓0+1
•На каком языке написан этот кусок кода?
C#
•Верен ли он синтаксически? Скомпилируется ли он?
Нет, переменная с именем «action» не может быть объявлена в параметрах лямбды/делегата, так как переменная с таким именем уже объявлена.
•Имеет ли данный код смысл? Что он делает?
В данном случае нет смысла. Пытается выполнить делегат, переданный в параметрах лямбды.
•Зачем такой код мог быть написан?
Для вызова callback-делегата, например, при асинхронности метода.
•Как можно улучшить этот код? (Как бы его написали Вы?)
В C# есть события. Я бы сделал событие с внятным именем.
•Приведите реальные варианты использования этого кода.
callback-метод
•Какие потенциальные проблемы могут возникнуть при его применении?
Если метод асинхронный, то мы должны синхронизироваться с потоком UI. На этот случай у делегата есть методы BeginInvoke, EndInvoke, callback-делегат в которых вызывается в том же потоке, из которого был вызван.
Total votes 3: ↑3 and ↓0+3
> •Верен ли он синтаксически? Скомпилируется ли он?
> Нет, переменная с именем «action» не может быть объявлена в параметрах лямбды/делегата, так как переменная с таким именем уже объявлена.

А если этот объявление поля класса, а не локальной переменной?

> •Как можно улучшить этот код? (Как бы его написали Вы?)
> В C# есть события. Я бы сделал событие с внятным именем.

А могли бы сюда примерчик запостить?
This comment wasn’t rated yet0
>> •Верен ли он синтаксически? Скомпилируется ли он?
>> Нет, переменная с именем «action» не может быть объявлена в параметрах лямбды/делегата, так как >>переменная с таким именем уже объявлена.

>А если этот объявление поля класса, а не локальной переменной?

Если мы напишем, например, в конструкторе:
this.action = action => action();
то синтаксически верно, но на Ваш вариант без this синтаксический анализатор также выдаст ошибку.

>> •Как можно улучшить этот код? (Как бы его написали Вы?)
>> В C# есть события. Я бы сделал событие с внятным именем.

>А могли бы сюда примерчик запостить?

В .NET есть класс System.ComponentModel.BackgroundWorker, который все иллюстрирует.
msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.aspx
This comment wasn’t rated yet0
> this.action = action => action();

Это не объявление поля, а его инициализация, я имел ввиду такой код:

    public class MyClass {
        Action<Action> action = (Action action) => { action(); };
        ...


Он скомпилируется без проблем.

И где здесь вы хотите использовать BackgroundWorker?

   Action<Action> action = (Action action) => { action(); };


И видимо придется третий раз повториться что реализовать асинхронную стратегию каждый может так как ему нравится и как походит для решения его конкретных задач, в соответствии с его принципами и опытом разработки. Представленная статья это не best parctices по реализации асинхронности, а об одном из множества способов изменения поведения класса, который не требует добавления новых сущностей.
Total votes 2: ↑1 and ↓10
Когда я реализовывал нечто подобное, второй тест иногда давал ложные сбои — в случае, когда работа все-таки выполняется быстро и флаг успевает изменится…
This comment wasn’t rated yet0
Все верно, поэтому тесты на асинхронное выполнение не должны полагаться «авось успеет», в этом случае должна быть четкая синхронизация потоков.
This comment wasn’t rated yet0
Ниже писали про паттерн, основанный на событиях — это собственно и есть решение…
This comment wasn’t rated yet0
У класса невнятный контракт.

void DoWorkAsync
void DoWork
event DoWorkCompleted

Сразу всё ясно и понятно (Event-based Asynchronous Pattern). А ребусы пусть кто-нибудь другой решает, если ему время не жалко.

Если асинхронность впоследствии не потребуется, то это должно решаться на более высоком уровне (этот уровень и будет стратегией), а не на уровне этого класса.

На работу я, может быть, вас и взял бы, но пометил для себя, что на первых порах надо с вами проводить инспекции дизайна.
Total votes 2: ↑2 and ↓0+2
> На работу я, может быть, вас и взял бы, но пометил для себя, что на первых порах надо с вами проводить инспекции дизайна.

А сколько бы денег дали? Ну так, ради интереса :)
Total votes 2: ↑1 and ↓10
Ехал экшн через экшн, видит экшн — в экшене экшен. Сунул экшн экшн в экшн. Экшн экшн экшн экшн.
Total votes 17: ↑16 and ↓1+15
Строка некомпилируемого кода в начале статьи, а в середине обыкновенная лямбда и некорректная работа с TPL…

Мне одному кажется что lambda это уже проза жизни? Неужели кого-то всё еще смущает их синтаксис? :)
This comment wasn’t rated yet0
Для тех, кто счел данный топик рекомендацией для реализации асинхронности. НИКОГДА не пишите так

(Action work) => { Task.Factory.StartNew(work); }


если не понимаете какими проблемами это может обернуться.

А пример давайте заменим на логирующую стратегию:

(Action work) => {
  Log("Task started");
  work(); 
  Log("Task completed");
}

This comment wasn’t rated yet0
Ну и раз уж пошла такая пьянка, давайте улучшим статью вместе и напишем правильную асинхронную стратегию, предлагаю начать, например, с такого варианта:

    public class AsyncStrategy {
        public void DoWork(Action work) {
            BackgroundWorker worker = new BackgroundWorker();
            worker.DoWork += (s, e) => { work(); };
            worker.RunWorkerCompleted += worker_RunWorkerCompleted;
            worker.RunWorkerAsync();
        }
        private void worker_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e) {
            if(e.Error != null) {
                //handle error here
            }
            BackgroundWorker worker = (BackgroundWorker)sender;
            worker.RunWorkerCompleted -= worker_RunWorkerCompleted;
        }
    }


Использование:

            MyWorker worker = new MyWorker();
            AsyncStrategy asyncStrategy = new AsyncStrategy();
            worker.SetDoWorkStrategy(asyncStrategy.DoWork);
This comment wasn’t rated yet0
Давайте не будем изобретать что-то «правильное», особенно если такого «правильного» нет в природе и все очень «it depends».
This comment wasn’t rated yet0
Т.е. Вы меня пытались убедить что мой пример абсолютно нерабочий, не смотря на то, что этот код безотказно работает уже долгое время, а мои доводы что «it depends» были названы невежеством и попыткой оправдать быдлокод. А теперь на попытку привести хоть какое-то более общее рабочее решение, Вы говорите что его не существует.
This comment wasn’t rated yet0
Есть плохие практики, которые лучше не использовать никогда. Есть хорошие практики, которые обкладываются десятком условий и приводятся в комплекте с этими условиями, когда они применимы. Было бы классно, если бы кто-то нашел бы время и описал лучшие практики в данном разрезе, с описанием когда какая применима.
This comment wasn’t rated yet0
Action action = (Action action) => { action(); };

Верен ли он синтаксически? Скомпилируется ли он?
— Нет. Во-первых, имя лямбда метода совпадает с именем принимаемой переменной. Во-вторых, лямбда методы не могут вызывать себя рекурсивно.
This comment wasn’t rated yet0
Как я уже сказал выше — выражение верно синтаксически и скомпилируется если это декларация поля класса, а не локальной переменной.
This comment wasn’t rated yet0
Sign up to leave a comment.

Articles

Show the best of all time

Your account

Sections

Information

Services

Support
© 2006–2024, Habr