为什么这段代码不好？

下面是rss reader的C＃代码，为什么这段代码不好？这个类将生成5个最近发布的帖子，按标题排序。你用什么来分析C＃代码？为什么这段代码不好？

static Story[] Parse(string content) 
    { 
     var items = new List<string>(); 
     int start = 0; 
     while (true) 
     { 

      var nextItemStart = content.IndexOf("<item>", start); 
      var nextItemEnd = content.IndexOf("</item>", nextItemStart); 
      if (nextItemStart < 0 || nextItemEnd < 0) break; 

      String nextItem = content.Substring(nextItemStart, nextItemEnd + 7 - nextItemStart); 
      items.Add(nextItem); 
      start = nextItemEnd; 
     } 

     var stories = new List<Story>(); 
     for (byte i = 0; i < items.Count; i++) 
     { 
      stories.Add(new Story() 
      { 
       title = Regex.Match(items[i], "(?<=<title>).*(?=</title>)").Value, 
       link = Regex.Match(items[i], "(?<=<link>).*(?=</link>)").Value, 
       date = Regex.Match(items[i], "(?<=<pubDate>).*(?=</pubdate>)").Value 
      }); 
     } 

     return stories.ToArray(); 
    }

来源

2012-09-26 Sofic

你能否描述你的意思是“坏”？虽然我可以说一件事，为什么不使用LINQ to XML来解析这个？ –

错误意味着垃圾代码或具有文体，功能，可维护性问题。 – Sofic

如果没有某种度量标准，某些东西不可能是好的或坏的。这些代码可以通过多种方式完成，但是如果不理解您想要最大化的内容，很难理解要改进的内容。 – madmik3

这是不好的，因为它是在解析XML的框架中使用字符串解析时的excellent classes。更好的是，有classes来处理RSS提要。

ETA：

很抱歉不能早点回答你的第二个问题。有很多工具可以分析C＃代码的正确性和质量。有可能介于编译一个巨大的名单，但这里有几个我日常使用的基础上，以帮助确保高质量代码：

StyleCop（代码格式化标准）
Resharper（惯用编程，疑难杂症捕）
FxCop（代码的正确性，标准遵守，惯用的编程）
Pex（白箱测试）
Nitriq（码质量度量）
NUnit（单元测试）

来源

2012-09-26 01:00:13

为什么不使用XmlReader或XmlDocument或LINQ to Xml？

来源

2012-09-26 00:56:55 yukaizhao

+1 - 完全 - 解析器我们是为代码试图解决的问题而设计的。另外，在n个字节的循环内执行RegEx匹配可能会很昂贵。像7这样的魔术数字恰好与代码中的字符串常量进一步对齐... – bryanmac

您不应该使用字符串函数和正则表达式来解析XML。 XML可以变得非常复杂，并且可以像XmlReader这样的真正的XML解析器可以处理的许多方式格式化，但会破坏简单的字符串解析代码。基本上：不要试图重新发明轮子（一个XML解析器），尤其是当你不知道轮子实际上有多复杂时。

来源

2012-09-26 00:58:58 PherricOxide

我认为代码最糟糕的是性能问题。您应该将xml字符串解析为XDocument（或类似的结构），而不是一次又一次使用正则表达式解析它。

来源

2012-09-26 00:59:06

对于初学者来说，它使用byte作为索引，而不是int（如果items中有更多的项目比byte能代表什么呢？）。它不使用惯用的C＃（请参阅user1645569的响应）。它也不必要地使用var而不是特定的数据类型（虽然这样更具风格，但对于我来说我不喜欢，因此按我的指标来看它并不理想（并且你没有给出其他指标））。

让我澄清一下我在说什么“不必要地使用var”：var本身并不坏，我并不是这样说的。我（主要）暗示这里的用法不是很一致。例如，明确声明start为int，但随后宣布nextItemEnd为var（将演绎到int）和想要自动推断变量的类型，并明确声明它之间分配nextItemEnd到start看来（我）像一个奇怪的混合物。我认为在start的声明中没有使用var是很好的（因为那么它的意图是整数还是浮点数并不完全清楚），但我（个人）认为它不会帮助任何人声明nextItemStart和nextItemEnd as var。我倾向于将var用于更复杂/更长的数据类型（类似于我在迭代器中使用C++中的auto，但不适用于“更简单”的数据类型）。

来源

2012-09-26 00:59:16 Cornstalks

-1的说明请吗？ – Cornstalks

我还没有投票，但建议使用var作为局部变量 –

@sleimanjneidi：谢谢，也许这就是为什么。我会澄清说''var'本身并不坏。我对'var'没有任何反应;我只是觉得它在这里用处不大（如果他将'start'声明为'int'，然后'nextItemEnd'声明为'var'，然后将'nextItemEnd'分配给'start'，我个人更喜欢他与显式声明类型一致，而不是在'int'和'var'之间切换）。 – Cornstalks

只是因为你正在改造一，使用Linq to xml相反，它是非常简单和clean.I相信你能做到以上三行代码，如果你使用Linq to XML，你的代码使用了大量的魔法数字（例如：7-n ..），这使得它不稳定和非通用的东西

来源

2012-09-26 01:01:53

为什么这段代码不好？

回答

相关问题