代码之家  ›  专栏  ›  技术社区  ›  marc

这个ASP.NET顾问知道他在做什么吗?

  •  9
  • marc  · 技术社区  · 17 年前

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

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

    首先,咨询负责人提供了以下伪代码:

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

    他接着说,会话变量的赋值是异步的,这似乎是不正确的。假定调用lookup函数可以异步执行某些操作,但这似乎不明智。

    考虑到这种所谓的异步性,他的理论是在引发重定向不可避免的threadabort异常之前,会话变量没有被分配。然后,这个错误阻止了其他页面显示正确的用户数据。

    其次,他给出了一个他推荐的编码最佳实践的例子。而不是写作:

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

    在某些情况下,从性能的角度来看,这当然是可行的,而且可能会更好。

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

    意见?

    22 回复  |  直到 15 年前
        1
  •  14
  •   John Rudy    17 年前

    我会同意的。这些人看起来很无能。

    (顺便说一句,我会检查“somepropertiessessionmanagementlookup”中是否使用了静态数据。看到了这个——带着行为 正如你所描述的 我几个月前继承了一个项目。当我们终于看到它时,那是一个令人头疼的时刻…希望我们能和写这封信的人面对面……)

        2
  •  19
  •   Serafina Brocious    17 年前

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

    我倾向于同意这一点。显然,你没有提供太多,但他们似乎不太称职。

        3
  •  12
  •   Randy    17 年前

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

        4
  •  8
  •   Mike Stone    17 年前

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

    通常,异步调用可以指定回调,这样您就可以确定结果是什么,或者操作是否成功。尽管会话的文档应该非常清楚,如果它实际上隐藏了异步调用,但是是的……看起来顾问不知道他在说什么…


    分配给会话索引器的方法调用不能是异步的,因为要异步获取值,必须使用回调…没有办法解决这个问题,所以如果没有显式回调,它肯定不是异步的(好吧,内部可能有异步调用,但是方法的调用方会认为它是同步的,因此如果方法内部(例如异步调用Web服务)是不相关的)。


    第二点,我认为这会更好,基本上保持相同的功能:

    int MyFunction(int x, int y)
    {
        if (y == 0)
        {
            // log it
            throw new DivideByZeroException("Divide by zero attempted!");
        }
    
        return x / y; 
    }
    
        5
  •  5
  •   Jon Skeet    17 年前

    首先,这看起来确实很奇怪。

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

        6
  •  4
  •   Robert Paulson    15 年前

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

    Session["UserID"] = SomeProprietarySessionManagementLookup();
    

    首先,从异步SomePropertySessionManagementLookup()分配返回值是行不通的。顾问守则可能看起来像:

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

    这个顾问并不是很满,但是他们写了一些错误代码。response.redirect()不会抛出ThreadAbort,如果专有方法是异步的,则会抛出ASP.NET 不知道等待 以便异步方法在ASP.NET本身保存会话之前回写会话。这可能就是为什么它有时工作有时不工作的原因。

    如果ASP.NET会话正在进行,它们的代码可能会工作,但是状态服务器或DB服务器不会工作。这取决于时间。

    我测试了以下内容。我们在开发中使用状态服务器。因为会话是在主线程完成之前写入的,所以此代码有效。

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

    第一步是让顾问的代码同步,因为 表演技巧 根本不起作用。如果修复了它,请让顾问使用 Asynchronous Programming Design Pattern

        7
  •  2
  •   Danimal    17 年前

    我部分同意他的观点——检查Y是否为零肯定比找出(昂贵的)异常要好。在我看来,出轨的问题是很过时的,但不管怎样。

    回复:异步sessionid buffoonery——可能是真的,也可能不是真的,但听起来好像顾问在冒烟。

        8
  •  1
  •   Community CDub    8 年前

    Cody's rule of thumb is dead right. 如果你不得不问,他可能不会。

    第二点似乎明显不正确。.NET的标准解释说,如果一个方法失败,它应该抛出一个异常,这似乎更接近于原始的异常;而不是consulstant的建议。假设异常准确地描述了故障。

        9
  •  1
  •   Simon    17 年前

    顾问首先创建了代码,对吗?但它不起作用。我想你已经把它们弄脏了。

    异步应答听起来像是bs,但其中可能有一些内容。大概他们已经提供了一个合适的解决方案以及描述他们自己创建的问题的伪代码。我更愿意根据他们的解决方案而不是他们对问题的表达来判断他们。如果他们的理解有缺陷,他们的新解决方案也不会奏效。你就会知道他们是白痴。(事实上,环顾四周,看看您是否在他们的代码的任何其他方面有类似的证据)

    另一个是代码样式问题。有很多不同的方法来处理这个问题。我个人不喜欢这种风格,但在某些情况下它是合适的。

        10
  •  1
  •   Cade Roux    17 年前

    异步上的错误。

    分配发生,然后页面重定向。函数可以异步启动某些东西并返回(甚至可以想象以它自己的方式更改会话),但是不管它返回什么,都必须在重定向之前的代码中分配。

    在任何低级代码中,甚至在高级函数中,这种防御性编码方式都是错误的,除非是特定的业务案例,即0、空或空字符串或任何应该以这种方式处理的内容——在这种情况下,它总是成功的(成功的标志是一种讨厌的代码味道),而不是例外。例外为例外。您不想通过溺爱函数的调用者来掩盖这样的行为。及早发现并抛出异常。我认为马奎尔在写了坚实的代码或麦康奈尔在完整的代码中覆盖了这一点。不管怎样,它闻起来很香。

        11
  •  1
  •   MusiGenesis    17 年前

    这家伙不知道他在做什么 . 显而易见的罪魁祸首就在这里:

    Session["UserID"] = SomeProprietarySessionManagementLookup();
    
        12
  •  0
  •   phreakre    17 年前

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

    …你的顾问听起来也不确定。

        13
  •  0
  •   Tom    17 年前

    以非异步方式存储在会话中。所以这不是真的,除非函数是异步的。但即便如此,由于它没有调用BeginCall,并且在完成时有一些东西需要调用,所以在会话行完成之前,下一行代码不会执行。

    对于第二个声明,虽然它可以被使用,但它并不完全是一个最佳实践,您有一些事情需要注意。你省去了抛出异常的成本,但是你不想知道你是在试图除以零而不是仅仅通过它吗?

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

        14
  •  0
  •   BlackWasp    17 年前

    很奇怪。在第二个项目上,它可能会更快,也可能不会更快。当然,它的功能不同。

        15
  •  0
  •   Duncan Smart    17 年前

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

    1. 问题出在somepropertysessionmanagementlookup正在做的任何事情上。
    2. 例外情况只有在被抛出时才是昂贵的。别害怕 try..catch ,但只应在 例外 情况。中频变量 y 不应该 然后是零 ArgumentOutOfRangeException 是合适的。
        16
  •  0
  •   Alvin    17 年前

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

        17
  •  0
  •   Thom    17 年前

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

    第二个问题也有点道理。它不是很惯用的——它是一个稍微有点颠倒的版本,比如int.triparse,它可以避免抛出许多异常导致的性能问题。但是,除非您将代码称为非常多的代码,否则它不太可能有显著的区别(与之相比,每页少一个数据库查询等)。在默认情况下,这当然不是你应该做的事情。

        18
  •  0
  •   Martin Brown    17 年前

    如果somePropertySessionManagementLookup();正在执行异步分配,则更可能如下所示:

    SomeProprietarySessionManagementLookup(Session["UserID"]);
    

    代码正在将结果分配给会话[“userid”]这一事实表明它不应该是异步的,并且应该在调用response.redirect之前获得结果。如果在计算结果之前返回SomePropertySessionManagementLookup,则它们仍然存在设计缺陷。

    抛出一个异常或者使用一个out参数是一个观点和环境的问题,在实际操作中,这并不意味着任何时候你都可以这样做。要使异常的性能命中成为一个问题,您需要多次调用函数,这可能本身就是一个问题。

        19
  •  0
  •   MusiGenesis    17 年前

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

    如果您所能找到的只是它们的.NET程序集(DLL和Exe),那么您仍然可以将它们分解成一些可读的源代码。我敢打赌,如果你仔细看代码,你会发现他们在专有的查找代码中使用静态变量。然后你会有一些非常具体的东西来展示你的老板。

        20
  •  0
  •   Shawn    16 年前

    整个回答流中充满了典型的程序员态度。它让我想起了乔尔的“你永远不应该做的事情”文章(从头重写)。我们对系统一无所知,除了有一个bug,还有一些人在网上发布了一些代码。有太多的未知,所以说“这个人不知道他在做什么”是荒谬的。

        21
  •  0
  •   Keith Adler    15 年前

    你可以轻而易举地把钱堆在购买他们服务的人身上,而不是堆在顾问身上。没有顾问是完美的,招聘经理也是完美的…但是在一天结束的时候,你应该采取的真正的方向是非常明确的: 你不应该试图找出错误,而应该把精力花在协作上寻找解决方案。 . 不管一个人在他们的角色和职责上有多熟练,他们肯定会有缺陷。如果您确定存在一种不合格的模式,那么您可以选择继续过渡到另一个资源,但是分配责任从来没有解决过历史上的单个问题。

        22
  •  -1
  •   Alf Zimmerman    17 年前

    在第二点上,我不会在这里使用异常。例外情况保留在例外情况下。
    然而,任何事物除以零肯定不等于零(至少在数学中),所以这是具体情况。

    推荐文章