该ASP.NET顾问知道他在做什么吗?
我们的子公司的IT部门有一家咨询公司,为他们编写了ASP.NET应用程序。现在,在混淆当前用户的身份时遇到了间歇性问题,并且已知该错误地向Joe显示了Bob的一些数据。
顾问们被带回进行故障排除,我们被邀请听取他们的解释。突出了两件事。
首先,顾问负责人提供了以下伪代码:
void MyFunction() { Session["UserID"] = SomeProprietarySessionManagementLookup(); Response.Redirect("SomeOtherPage.aspx"); }
他继续说,会话变量的分配是异步的,这似乎是不正确的。授予对lookup函数的调用可以异步执行某些操作,但这似乎是不明智的。
考虑到所谓的异步性,他的理论是在引发重定向不可避免的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; }
从性能的角度来看,这肯定会起作用,并且在某些情况下可能会更好。
但是,从这些以及其他讨论的观点来看,在我们看来,这个团队并不精通技术。
意见?
解决方案
经验法则:如果我们需要询问顾问是否知道他在做什么,他可能不知道;)
我倾向于在这里同意。显然,我们没有提供太多,但他们似乎并不称职。
首先,这确实看起来很奇怪。
在第二种情况下,尝试避免被0除是完全合理的,并且避免很简单,这是合理的。但是,仅在某些情况下使用out参数表示成功是合理的,例如int.TryParse和DateTime.TryParseExact,在这些情况下,调用者无法轻松确定其参数是否合理。即使这样,返回值通常还是成功/失败,而out参数是该方法的结果。
如果顾问编写的应用程序应该能够跟踪用户,并且仅向正确的用户显示正确的数据,但它没有这样做,那么显然是有问题的。好的顾问会发现问题并解决。一个糟糕的顾问会告诉我们这是异步的。
我会同意的。这些家伙似乎很无能。
(顺便说一句,我要检查一下是否在" SomeProprietarySessionManagementLookup"中,他们使用的是静态数据。看到了这个-行为与我们几个月前继承的项目中描述的完全一样。终于看到了... ...希望我们能与编写它的人面对面...)
我部分同意他的观点-最好将y检查为零,而不是捕获(昂贵的)异常。成功似乎对我而言确实过时了,但无论如何。
回复:异步sessionid buffoonery-可能正确,也可能不正确,但听起来顾问正在冒烟掩盖。
科迪的经验法则是正确的。如果我们必须要问,他可能没有。
似乎第二点显然是不正确的。 .NET的标准解释说,如果方法失败,则应引发异常,该异常似乎更接近于原始异常。不是领事的建议。假设异常是准确而具体地描述了故障。
顾问首先创建了代码,对吗?而且它不起作用。我认为我们已经在它们上面积满了很多污垢。
异步答案听起来像BS,但其中可能包含某些内容。大概他们提供了一个合适的解决方案以及伪代码来描述他们自己创建的问题。我会更倾向于根据他们的解决方案而不是他们对问题的表达来判断他们。如果他们的理解存在缺陷,那么他们的新解决方案也不起作用。然后,我们将知道它们是白痴。 (事实上,环顾四周,看看我们是否已经在其代码的任何其他区域中找到了类似的证明)
另一个是代码样式问题。有很多不同的方法来解决这个问题。我个人不喜欢这种风格,但是在某些情况下它是合适的。
第二点,我在这里不使用异常。例外情况保留例外。
但是,任何东西都被零除肯定不等于零(至少在数学上是这样),因此这要视情况而定。
典型的"顾问"锁:
- 问题在于SomeProprietarySessionManagementLookup在做什么
- 只有抛出异常,它们才代价高昂。不要害怕
try..catch
,但是仅在特殊情况下才会发生抛出。如果变量y不应该为零,则应该使用ArgumentOutOfRangeException。
我必须同意约翰·鲁迪。我的直觉告诉我问题出在SomeProprietarySessionManagementLookup()中。
..并且顾问听起来不确定。
他们在异步上是错误的。
分配发生,然后页面重定向。该函数可以异步启动并返回(甚至可以想象以自己的方式更改Session),但是返回的内容必须在重定向之前提供的代码中进行分配。
在任何低级代码甚至高级函数中,这种防御性编码样式都是错误的,除非在特定的业务案例中,应该以这种方式处理0或者NULL或者空字符串或者任何其他方式,成功(该成功标志是一种讨厌的代码味道),也不例外。例外是例外。我们不想通过缠扰函数的调用者来掩盖这样的行为。早点抓东西并抛出异常。我认为Maguire在《编写扎实的代码》或者《 McConnell在Code Complete》中对此进行了介绍。无论哪种方式,它都闻起来。
以非异步方式存储在会话中。因此,除非该函数是异步的,否则情况并非如此。但是即使如此,由于它没有调用BeginCall且在完成时需要调用某些内容,因此下一行代码要在Session行完成后才能执行。
对于第二种说法,虽然可以使用,但这并不是最佳实践,我们需要注意一些事项。我们节省了引发异常的开销,但是我们是否不想知道我们正在尝试除以零而不是仅仅跳过零?
我认为这根本不是一个可靠的建议。
在异步方面,唯一可以实现的方法是,如果正在进行的分配实际上在Session上有一个索引器设置程序,该设置程序隐藏了一个异步调用,而没有指示成功/失败的回调。这似乎是一个可怕的设计选择,并且看起来像我们框架中的核心类,所以我发现它极不可能。
通常,异步调用具有指定回调的方法,因此我们可以确定结果是什么,或者操作是否成功。 Session的文档应该很清楚,尽管它实际上是否隐藏了异步调用,但是是的...看起来好像顾问不知道他在说什么...
分配给会话索引器的方法调用不能是异步的,因为要异步获取一个值,我们必须使用回调...不能解决这个问题,因此,如果没有显式的回调,则绝对不是异步的。 ,内部可能会有一个异步调用,但是该方法的调用者会认为它是同步的,因此,例如内部该方法异步调用Web服务就无关紧要。
对于第二点,我认为这会更好,并且基本上保留相同的功能:
int MyFunction(int x, int y) { if (y == 0) { // log it throw new DivideByZeroException("Divide by zero attempted!"); } return x / y; }
很奇怪。在第二项上,它可能会更快,也可能不会更快。它当然不是相同的功能。
我猜顾问建议使用状态变量而不是异常进行错误处理是更好的做法?我不同意人们多久忘记一次或者懒得对返回值进行错误检查?同样,通过/失败变量也不提供信息。除了被零除(例如整数x / y太大或者x是NaN)以外,还有更多的事情可能出错。当事情出错时,状态变量不能告诉我们哪里出错了,但是异常可以。例外是例外情况,除以零或者NaN绝对是例外情况。
这个家伙不知道他在做什么。明显的罪魁祸首就在这里:
Session["UserID"] = SomeProprietarySessionManagementLookup();
会议的事情是可能的。毫无疑问,这是一个错误,但是很可能是写入到达了下一次读取之后我们正在使用的任何自定义会话状态提供程序。会话状态提供程序API可以进行锁定以防止发生这种情况,但是如果实现者只是忽略了所有这些,则顾问可能会说实话。
第二个问题也很有效。它不是惯用语言,它是int.TryParse之类的东西的略有相反的版本,可以避免因抛出大量异常而导致的性能问题。但是除非我们称该代码为难,否则它不会带来明显的改变(相比之下,每页少一个数据库查询,等等)。默认情况下,这当然不是我们应该执行的操作。
如果我们使用的是内置提供程序,则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会话正在进行中,则它们的代码可能会起作用,但是状态服务器或者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);
第一步是使顾问使他们的代码同步,因为他们的性能技巧根本不起作用。如果可以解决问题,请顾问使用"异步编程设计模式"正确实施
如果SomeProprietarySessionManagementLookup();正在执行异步分配,它更可能看起来像这样:
SomeProprietarySessionManagementLookup(Session["UserID"]);
代码将结果分配给Session [" UserID"]的事实表明,这不应该是异步的,应该在调用Response.Redirect之前获得结果。如果SomeProprietarySessionManagementLookup在计算结果之前返回,则它们仍然存在设计缺陷。
引发异常或者使用out参数是一种见解和情况,在实际实践中,无论我们采用哪种方式,都不会像一堆豆子一样。为了使异常对性能的影响成为问题,我们需要多次调用该函数,而这本身可能就是一个问题。
如果顾问在服务器上部署了他们的ASP.NET应用程序,则他们可能已经以未编译的形式部署了它,这意味着我们将看到一堆* .cs文件。
如果我们能找到的只是它们的已编译.NET程序集(DLL和EXE),那么我们仍然应该能够将其反编译为某种可读的源代码。我敢打赌,如果我们浏览这些代码,就会在其专有的查找代码中使用静态变量来找到它们。然后,我们将有一些非常具体的东西来向老板展示。
整个答案流充满了典型的程序员态度。这让我想起了Joel的"我们永远不应该做的事"(从头开始重写)。除了有一个bug外,我们对系统一无所知,有些人在线上发布了一些代码。未知数太多,说"这个家伙不知道自己在做什么"真是荒谬。
除了堆积在顾问身上之外,我们还可以堆积在采购他们的服务人员身上。没有顾问是完美的,招聘经理也不是完美的……但是到了最后,我们应该采取的真正方向是非常明确的:不要试图发现错误,而应该花费精力进行协作以找到解决方案。无论某人在其角色和职责上有多熟练,他们肯定都会有缺陷。如果我们确定存在某种不合时宜的模式,那么我们可以选择过渡到其他资源,但是分配指责从未解决过历史上的单个问题。