Этот консультант ASP.NET знает, что он делает?

Question

ИТ-отдел нашей дочерней компании попросил консалтинговую компанию написать им приложение ASP.NET. Теперь у него периодически возникают проблемы со смешиванием того, кто является текущим пользователем, и, как известно, ошибочно показывает Джо некоторые данные Боба.

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

Сначала руководитель консультанта предоставил этот псевдокод:

void MyFunction()
{
    Session["UserID"] = SomeProprietarySessionManagementLookup();
    Response.Redirect("SomeOtherPage.aspx");
}

Далее он сказал, что присвоение переменной сеанса асинхронно, что казалось неверным. Конечно, вызов функции поиска может сделать что-то асинхронно, но это кажется неразумным.

Учитывая эту предполагаемую асинхронность, его теория заключалась в том, что переменная сеанса не назначалась до того, как было сгенерировано неизбежное исключение ThreadAbort перенаправления. Эта ошибка не позволила SomeOtherPage отобразить правильные данные пользователя.

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

int MyFunction(int x, int x)
{
    try 
    {
        return x / y; 
    }
    catch(Exception ex)
    {
        // log it
        throw;
    }
}

Техника, которую он рекомендовал, была:

  int MyFunction(int x, int y, out bool isSuccessful)
  {
    isSuccessful = false;

    if (y == 0)
        return 0;

    isSuccessful = true;

    return x / y;
  }

Это, безусловно, сработает и в некоторых ситуациях может быть лучше с точки зрения производительности.

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

Мнения

Answer 1

Практическое правило: если вам нужно спросить, знает ли консультант, что он делает, он, вероятно, не знает;)

И я склонен согласиться здесь. Очевидно, вы не предоставили много, но они не кажутся очень компетентными.

Answer 2

Я бы согласился. Эти парни кажутся совершенно некомпетентными.

(Кстати, я бы проверил, используют ли статические данные в "SomePprietarySessionManagementLookup"). Это видно по поведению , точно так же, как вы описываете в проекте, который я унаследовал несколько месяцев назад. Это был тот самый момент, когда мы наконец увидели это ... И хотелось бы встретиться лицом к лицу с парнями, которые это написали ...)

Answer 3

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

Answer 4

В асинхронной части единственный способ, который может быть верным, состоит в том, если выполняемое там присваивание фактически является установщиком индексатора в сеансе, который скрывает асинхронный вызов без обратного вызова, указывающего на успех / сбой. Это может показаться УЖАСНЫМ выбором дизайна, и он выглядит как базовый класс в вашей среде, поэтому я считаю его маловероятным.

Обычно асинхронные вызовы имеют возможность указать обратный вызов, чтобы вы могли определить, каков результат или была ли операция успешной. Документация для Session должна быть достаточно ясной, если она действительно скрывает асинхронный вызов, но да ... не похоже, что консультант знает, о чем говорит ...

---

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

---

Во-вторых, я думаю, что это было бы намного лучше, и, по сути, сохранил бы ту же функциональность:

int MyFunction(int x, int y)
{
    if (y == 0)
    {
        // log it
        throw new DivideByZeroException("Divide by zero attempted!");
    }

    return x / y; 
}

Answer 5

Во-первых, это действительно кажется странным.

Во втором случае разумно попытаться избежать деления на 0 - его можно полностью избежать, и это просто. Однако использование параметра out для указания успеха целесообразно только в определенных случаях, таких как int.TryParse и DateTime.TryParseExact - где вызывающий не может легко определить, являются ли их аргументы разумными. Даже тогда возвращаемое значение обычно является успехом / неудачей, а выходной параметр - результатом метода.

Answer 6

Сеансы Asp.net, если вы используете встроенных провайдеров, случайно не дадут вам чужой сеанс. SomeProprietarySessionManagementLookup() является вероятным виновником и возвращает плохие значения или просто не работает.

Session["UserID"] = SomeProprietarySessionManagementLookup();

Прежде всего, присвоение возвращаемого значения из асинхронно SomePprietarySessionManagementLookup () просто не будет работать. Код консультанта, вероятно, выглядит так:

public void SomeProprietarySessionManagementLookup()
{
    // do some async lookup
    Action<object> d = delegate(object val)
    {
        LookupSession(); // long running thing that looks up the user.
        Session["UserID"] = 1234; // Setting session manually
    };

    d.BeginInvoke(null,null,null);               
}

Консультант не полностью владеет BS, но они написали некоторый глючный код. Response.Redirect () генерирует ThreadAbort, и если проприетарный метод является асинхронным, asp.net не знает, ждать для асинхронного метода, чтобы записать обратно в сеанс, прежде чем сам asp.net сохранит сессия. Вероятно, поэтому иногда это работает, а иногда нет.

Их код может работать, если сеанс asp.net находится в процессе, но сервер состояний или сервер баз данных не будут. Это зависит от времени.

Я проверял следующее. Мы используем государственный сервер в разработке. Этот код работает, поскольку сеанс записывается до завершения основного потока.

Action<object> d = delegate(object val)
{
    System.Threading.Thread.Sleep(1000);  // waits a little
    Session["rubbish"] = DateTime.Now;
};

d.BeginInvoke(null, null, null);
System.Threading.Thread.Sleep(5000);      // waits a lot

object stuff = Session["rubbish"];
if( stuff == null ) stuff = "not there";
divStuff.InnerHtml = Convert.ToString(stuff);

Этот следующий фрагмент кода не работает , поскольку сеанс уже был сохранен обратно на сервер состояний к тому времени, когда асинхронный метод доходит до установки значения сеанса.

Action<object> d = delegate(object val)
{
    System.Threading.Thread.Sleep(5000);  // waits a lot
    Session["rubbish"] = DateTime.Now;
};

d.BeginInvoke(null, null, null);

// wait removed - ends immediately.
object stuff = Session["rubbish"];
if( stuff == null ) stuff = "not there";
divStuff.InnerHtml = Convert.ToString(stuff);

Первый шаг для консультанта - сделать свой код синхронным, потому что его трюк производительности не сработал вообще. Если это исправит, попросите консультанта правильно реализовать, используя Шаблон проектирования асинхронного программирования

Answer 7

Я согласен с ним частично - определенно лучше проверять y на ноль, чем ловить (дорогое) исключение. Мне кажется, что «Успех» действительно устарел, но неважно.

re: асинхронная сессионная баффонерия - может быть или не быть правдой, но похоже, что консультант выпускает дым для укрытия.

Answer 8

Консультанты создали код в первую очередь, верно? И это не работает. Я думаю, что у вас уже есть немного грязи на них.

Асинхронный ответ звучит как BS, но в нем может быть что-то. Предположительно они предложили подходящее решение, а также псевдокод, описывающий проблему, которую они сами создали. Я был бы более искушен судить их по их решению, а не по выражению проблемы. Если их понимание ошибочно, их новое решение тоже не сработает. Тогда ты узнаешь, что они идиоты. (На самом деле посмотрите вокруг, чтобы увидеть, есть ли у вас аналогичные доказательства в каких-либо других областях их кода)

Другая проблема связана со стилем кода. Есть много разных способов справиться с этим. Мне лично не нравится этот стиль, но будут обстоятельства, при которых он подходит.

Answer 9

Эмпирическое правило Коди совершенно верно. Если вам нужно спросить, он, вероятно, нет.

Кажется, что пункт два явно неверен. Стандарты .NET объясняют, что в случае сбоя метода он должен вызвать исключение, которое кажется ближе к оригиналу; не предложение консультанта. Предполагая, что исключение точно и конкретно описывает сбой.

Answer 10

Они не правы в асинхронном.

Назначение происходит, а затем страница перенаправляется. Функция может запускать что-то асинхронно и возвращать (и может даже изменить Session по-своему), но все, что она возвращает, должно быть назначено в коде, который вы передали перед перенаправлением.

Они ошибаются в этом стиле защитного кодирования в любом низкоуровневом коде и даже в высокоуровневой функции, если только это не конкретный бизнес-случай, когда 0 или NULL или пустая строка или что-либо еще должно обрабатываться таким образом, - в котором случай, он всегда успешен (этот успешный флаг - неприятный запах кода) и не исключение. Исключения для исключений. Вы не хотите маскировать поведение, подобное этому, с помощью функций, вызывающих функции. Лови вещи рано и бросай исключения. Я думаю, что Магуайр рассказал об этом в «Написание твердого кода» или «Макконнелл» в «Коде завершено». В любом случае, пахнет.