这段代码有什么好处或坏处？ [英] What is Good or Bad with this code?

问题描述

我之前发过这个问题，但是我偶然删除了它。



我被要求查看下面的代码片段并评论代码的优缺点。我一整天都在做家务，我的大脑都被炸了。请帮助

  public   void  DoSomething（ int  CustomerID）
 {
  try  
 {
  var  x = GetPersonByID（CustomerID）; 
  if （x.Name.Length（）>   25 ）
 {
 x.Name = x.Name.Substring（ 0 ， 25 ）;   
} 
 DoSomethingElse（x）; 
} 
  catch （例外情况）
 {
  throw   new 异常（ 找不到客户！< /跨度>）; 
} 
 Console.WriteLine（ 完成某事！）; 
} 

解决方案

错误：您使用立即常量： 25，找不到客户等。它使代码不受支持。如果您需要将25更改为其他内容怎么办？您必须在两个或更多地方更改它。如果您忘记在一个地方更改它怎么办？这只是灾难。对于常量，在某些情况下使用显式声明的 const - 资源。除了例外情况，此问题尤其严重。在最终处理异常时，如何过滤异常，停止传播。异常处理旨在通过异常类型来实现。因此，您需要一种特殊的异常类型。它不应该来自 System.Exception ，而是来自 System.ApplicationException 或更多派生类型（只是约定，虽然不重要，但仍然如此）。其他问题：某些名称不好： CustomerID ，作为方法参数，应该大写为 customerId （Microsoft命名约定非常好）。名称 x 和 ex 太短。所有名称中只使用拼写正确的英文名称是个好主意;它可以大大简化查找它们。



好：你重新抛出异常而不阻止进一步的异常传播是件好事。你真的应该让他们去处理很少的，一些策略性的设置点在堆栈的某个地方（我称之为能力点）。最终，它们都应该在每个线程的堆栈顶部处理，用于UI - 在应用程序的主事件循环的特殊点。您应该只将异常类型更改为某种自定义类型，例如 CustomerNotFoundException ;我已经在坏部分解释了它。



祝你好运，

-SA

你需要做一些思考 - 这里有一些你可能想要考虑的事情



更容易阅读

  if （a = b）
 {
 dostuff ; 
} 

或

  if （a = b）
 {
 dostuff; 
} 

DoSomething是什么意思？ Wuld有一个更好的名字是TrimCustomerName或者它本来要做什么吗？



当你捕获异常时，它是否只会抛出异常客户不存在？如果数据库损坏，或网络故障等怎么办？



它试图将名称修改为25个字符 - 为什么？一些评论可能有帮助。



它使用数字25两次 - 这意味着需求变化，需要更改为30，需要在两个地方更改 - 你怎么能得到那么只有一个地方可以改变它吗？



var有效c＃=在这种情况下，x变量将是GetPersonByID方法返回的类型，所以它可能是一个人。



说到哪个-x不是一个非常有意义的名字...



这应该足以让老师高兴吗？

I posted this question earlier, but I deleted it by accident.

I have been asked to review the code snippet below and comment on what is good or bad about the code. I've been doing home work all day and my brain is fried. Please help

public void DoSomething(int CustomerID)
{
try
{
var x = GetPersonByID(CustomerID);
if (x.Name.Length() > 25)
{
x.Name = x.Name.Substring(0, 25);
}
DoSomethingElse(x);
}
catch (Exception ex)
{
throw new Exception("Customer not found!");
}
Console.WriteLine("done doing something!");
}

解决方案

Bad: You use immediate constants: 25, "Customer not found", etc. It makes the code unsupportable. What if you need to change 25 to something else? You will have to change it in two or more places. What if you forget to change it in one place? This is just the disaster. For constants, use explicitly declared const, in some cases — resources. This problem is especially bad with exceptions. How would you filter exceptions when you handle them finally, stopping their propagation. The exception handling is designed to do it by exception types. So, you would need a special exception type. It should be derived not from System.Exception, but from System.ApplicationException or more derived type (just the convention, not critically important, but nevertheless). Other problems: some names are bad: CustomerID, as a method parameter, should be capitalized as customerId (Microsoft naming conventions are pretty good). The names as x and ex are too short. It's a good idea to use only correctly spelled English names in all names; it may greatly simplify finding them.

Good: It's good that you are re-throwing exception and not blocking further exception propagation. You really should let them go and handle in very few, some strategically set points somewhere up the stack (I call them "competency points"). Ultimately, they all should be handled on top of stack for each thread, for UI — in a special points of the main event loop of the application. You only should have changed the exception type to some custom type, like CustomerNotFoundException; I already explained it in the "Bad" section.

Good luck,

—SA

You need to do some thinking - here are some things you may want to think about

Is it easier to read

if (a=b)
{
dostuff;
}

or

if (a=b)
{
    dostuff;
}

What does it mean to DoSomething? Wuld a better name be "TrimCustomerName" or whatever it is meant to be doing?

When you are catching an exception , is it only going to throw an exception if the customer doesn't exist? what if the database is corrupt, or the network down etc.?

it's trying to trim the name to 25 characters - why? Some comments might help.

it's using the number 25 twice - which means of requirements change and it needs to be changed to 30, it needs to be changed in two places - how could you get around that so there is only one place to change it?

var is valid c# = in this case the x variable will be of the type returned by the GetPersonByID method, so it will be a Person, probably.

Speaking of which- x isn't a very meaningful name...

That should be enough to keep the teacher happy?

这篇关于这段代码有什么好处或坏处？的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！