9

我们一家子公司的 IT 部门让一家咨询公司为他们编写了一个 ASP.NET 应用程序。现在它在混淆当前用户是谁并且已知错误地向 Joe 显示 Bob 的一些数据时遇到了间歇性问题。

顾问被带回来进行故障排除,我们被邀请听取他们的解释。有两件事突出。

首先,顾问领导提供了这个伪代码:

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;
  }

这肯定会奏效,并且在某些情况下从性能的角度来看可能会更好。

然而,从这些和其他讨论点来看,在我们看来,这个团队在技术上并不精通。

意见?

4

22 回答 22

19

经验法则:如果您需要询问顾问是否知道他在做什么,他可能不知道;)

我倾向于同意这里。显然你没有提供太多,但他们似乎不是很能干。

于 2008-10-02T20:39:55.643 回答
14

我会同意的。这些家伙似乎很无能。

(顺便说一句,我会检查他们是否在“SomeProprietarySessionManagementLookup”中使用静态数据。看到这一点 - 行为与你在几个月前继承的项目中描述的完全一样。我们终于看到了......并希望我们能与写它的人面对面......)

于 2008-10-02T20:43:06.977 回答
12

如果顾问编写的应用程序应该能够跟踪用户并且只向正确的用户显示正确的数据而它没有这样做,那么显然有问题。一个好的顾问会发现问题并解决它。一个糟糕的顾问会告诉你这是异步的。

于 2008-10-02T20:42:24.280 回答
8

在异步部分,唯一可行的方法是,如果正在进行的分配实际上是 Session 上的一个索引器设置器,它隐藏了一个异步调用,没有回调指示成功/失败。这似乎是一个可怕的设计选择,它看起来像你框架中的一个核心类,所以我觉得它不太可能。

通常异步调用有一种方法可以指定回调,这样您就可以确定结果是什么,或者操作是否成功。Session 的文档应该很清楚,如果它实际上隐藏了一个异步调用,但是是的......看起来顾问不知道他在说什么......


分配给会话索引器的方法调用不能是异步的,因为要异步获取值,您必须使用回调......没有办法解决这个问题,所以如果没有显式回调,它绝对不是异步的(嗯,内部可能有一个异步调用,但是方法的调用者会认为它是同步的,因此如果方法内部例如异步调用 Web 服务,则无关紧要)。


对于第二点,我认为这会好得多,并且基本上保持相同的功能:

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

    return x / y; 
}
于 2008-10-02T20:57:25.493 回答
5

对于第一点,这确实看起来很奇怪。

在第二个问题上,尽量避免除以 0 是合理的——这是完全可以避免的,而且避免很简单。但是,使用 out 参数表示成功仅在某些情况下是合理的,例如 int.TryParse 和 DateTime.TryParseExact - 调用者无法轻松确定其参数是否合理。即使这样,返回值通常是成功/失败,而 out 参数是方法的结果。

于 2008-10-02T20:40:57.653 回答
4

Asp.net 会话,如果您使用的是内置提供程序,不会意外给您其他人的会话。SomeProprietarySessionManagementLookup()是可能的罪魁祸首,并且返回错误的值或只是不工作。

Session["UserID"] = SomeProprietarySessionManagementLookup();

首先,从异步 SomeProprietarySessionManagementLookup() 分配返回值是行不通的。顾问代码可能如下所示:

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);

第一步是让顾问让他们的代码同步,因为他们的性能技巧根本不起作用。如果解决了问题,请让顾问使用异步编程设计模式正确实施

于 2008-10-02T21:46:14.737 回答
2

我部分同意他的观点——检查 y 是否为零肯定比捕捉(昂贵的)异常要好。out bool isSuccessful 对我来说似乎真的过时了,但无论如何。

回复:异步 sessionid 小丑 - 可能是也可能不是,但听起来顾问正在冒烟作为掩护。

于 2008-10-02T20:43:09.270 回答
1

科迪的经验法则是完全正确的。 如果你不得不问,他可能不会。

似乎第二点显然是不正确的。.NET 的标准解释说,如果一个方法失败,它应该抛出一个异常,这似乎更接近原来的;不是顾问的建议。假设异常准确且具体地描述了失败。

于 2008-10-02T20:44:49.890 回答
1

顾问首先创建了代码,对吗?它不起作用。我想你已经在他们身上沾满了污垢。

异步答案听起来像废话,但其中可能有一些东西。大概他们已经提供了一个合适的解决方案以及描述他们自己创建的问题的伪代码。我更倾向于根据他们的解决方案而不是他们对问题的表达来评判他们。如果他们的理解有缺陷,他们的新解决方案也将不起作用。然后你就会知道他们是白痴。(事实上​​,看看你是否已经在他们代码的任何其他区域有类似的证明)

另一个是代码风格问题。有很多不同的方法可以应对这种情况。我个人不喜欢那种风格,但会有适合的情况。

于 2008-10-02T20:48:44.750 回答
1

他们在异步上是错误的。

分配发生,然后页面重定向。该函数可以异步启动某些内容并返回(甚至可以想象以自己的方式更改 Session),但无论它返回什么,都必须在您在重定向之前提供的代码中分配。

他们在任何低级代码甚至高级函数中的防御性编码风格都是错误的,除非它是一个特定的业务案例,即 0 或 NULL 或空字符串或任何应该以这种方式处理的东西 - 在这种情况下,它是总是成功(成功的标志是令人讨厌的代码味道)而不是例外。例外是例外。你不想通过溺爱函数的调用者来掩盖这样的行为。尽早发现并抛出异常。我认为 Maguire 在编写 Solid Code 或 McConnell 在 Code Complete 中介绍了这一点。无论哪种方式,它都闻起来了。

于 2008-10-02T20:52:49.497 回答
1

这家伙不知道自己在做什么。明显的罪魁祸首就在这里:

Session["UserID"] = SomeProprietarySessionManagementLookup();
于 2008-10-02T21:19:53.643 回答
0

典型的“顾问”胡说八道:

  1. 问题在于 SomeProprietarySessionManagementLookup 正在做什么
  2. 只有抛出异常才会昂贵。不要害怕,但只有在特殊try..catch情况下才会发生抛出。如果变量应该为零,那么 an将是合适的。y ArgumentOutOfRangeException
于 2008-10-02T20:50:35.557 回答
0

我必须同意约翰鲁迪的观点。我的直觉告诉我问题出在 SomeProprietarySessionManagementLookup() 中。

..而且您的顾问听起来并不确定。

于 2008-10-02T20:51:00.407 回答
0

以非异步方式存储在 Session 中。因此,除非该函数是异步的,否则这是不正确的。但即便如此,由于它没有调用 BeginCall 并且在完成时有要调用的内容,所以在 Session 行完成之前,下一行代码不会执行。

对于第二个语句,虽然可以使用它,但它并不是最佳实践,您需要注意一些事项。您节省了抛出异常的成本,但您不想知道您正在尝试除以零而不是仅仅移动过去吗?

我认为这根本不是一个可靠的建议。

于 2008-10-02T20:53:30.353 回答
0

很奇怪。在第二个项目上,它可能会或可能不会更快。虽然它肯定不是相同的功能。

于 2008-10-02T21:00:29.853 回答
0

我猜您的顾问建议使用状态变量而不是异常进行错误处理是一种更好的做法?我不同意。人们多久忘记或懒得对返回值进行错误检查?此外,通过/失败变量不提供信息。除了除以零之外,还有更多的事情可能出错,比如整数 x/y 太大或 x 是 NaN。当事情出错时,状态变量不能告诉你出了什么问题,但异常可以。Exception 是例外情况,除以零或 NaN 绝对是例外情况。

于 2008-10-02T21:16:49.440 回答
0

会话的事情是可能的。毫无疑问,这是一个错误,但可能是在下一次读取之后,写入到达了您正在使用的任何自定义会话状态提供程序。会话状态提供者 API 提供锁定以防止此类事情发生,但如果实施者只是忽略了所有这些,那么您的顾问可能会说实话。

第二个问题也有点道理。这不是很惯用 - 它是 int.TryParse 之类的稍微颠倒的版本,用于避免因抛出大量异常而导致的性能问题。但是,除非您大量调用该代码,否则它不太可能产生明显的差异(相比之下,每页减少一个数据库查询等)。这当然不是您默认应该做的事情。

于 2008-10-02T21:39:17.437 回答
0

If SomeProprietarySessionManagementLookup(); 正在执行异步分配,它更可能看起来像这样:

SomeProprietarySessionManagementLookup(Session["UserID"]);

代码将结果分配给 Session["UserID"] 的事实表明它不应该是异步的,并且应该在调用 Response.Redirect 之前获得结果。如果 SomeProprietarySessionManagementLookup 在计算其结果之前返回,则无论如何它们都存在设计缺陷。

抛出异常或使用 out 参数是一个观点和情况的问题,在实际实践中,无论你怎么做,都不会成为一堆豆子。要使异常对性能的影响成为一个问题,您需要大量调用该函数,这本身可能就是一个问题。

于 2008-10-03T13:42:15.673 回答
0

如果顾问在您的服务器上部署了他们的 ASP.NET 应用程序,那么他们可能已经以未编译的形式部署了它,这意味着您可以查看一堆浮动的 *.cs 文件。

如果您只能找到他们的已编译的 .NET 程序集(DLL 和 EXE),那么您仍然应该能够将它们反编译成一些可读的源代码。我敢打赌,如果您查看代码,您会发现它们在专有查找代码中使用静态变量。然后,您将有一些非常具体的东西向您的老板展示。

于 2008-10-06T23:02:23.420 回答
0

整个答案流充满了典型的程序员态度。它让我想起了 Joel 的“你不应该做的事情”文章(从头开始重写。)我们对系统一无所知,除了有一个错误,有人在网上发布了一些代码。未知数太多,说“这家伙不知道自己在做什么”是很可笑的。

于 2009-01-26T02:20:20.790 回答
0

而不是堆积在顾问身上,你可以很容易地堆积在购买他们服务的人身上。没有顾问是完美的,招聘经理也不是……但归根结底,你应该采取的真正方向非常明确:与其试图找出错误,不如将精力投入到协作寻找解决方案上。无论某人在他们的角色和职责上多么熟练,他们肯定会有不足之处。如果您确定存在一种不称职的模式,那么您可能会选择过渡到另一个资源,但在历史上,指责从来没有解决过一个问题。

于 2010-03-08T20:35:12.637 回答
-1

关于第二点,我不会在这里使用例外。例外是为特殊情况保留的。
但是,任何东西除以零肯定不等于零(至少在数学上),所以这将是具体情况。

于 2008-10-02T20:49:20.660 回答