代码复审

TeamSHIT

　　我们小组阅读的是SuperBrother组的代码，按照代码大全的内容作为评价标准。

　　复审结果：代码大全会就一个具体问题给出几种比较优秀的规范，SB的代码大体上符合代码大全里说的某一种标准。具体地说，变量名取得都比较规范，按照变量名的前缀、字面意思以及一些必要的注释，基本能清楚编码人员设置这个变量的作用；变量的使用符合就近原则，没有出现一个变量的生存周期很长导致使用的时候忘记它之前设置的值这类问题；程序的结构良好，将实现某一个具体功能的代码群放置在一起；抽象了大量接口，每个变量都写了get和set函数，设计了很多类，保证每个类完成一个单一的功能。以上4点非常良好的保证了其工程的开发性和可维护性，特别是第4点，要养成一个习惯需要大量的编程经历，特别难能可贵。基于这些，我们小组给SuperBrother的代码打85分。

 

一下是一些问题：

1.注释写得太随性，有些代码会添加注释，有些代码近300行却没有一个说明文字。

这是有注释的代码段，通过一些说明文字，这个分支结构很容易理解理解：

 else // 把每个单词丢入相应的bucket中，其中新词丢入_newWordList，旧词丢入_oldWordList
            {
                _buckets.Add(new Bucket());
                for (int i = 0; i < _words.Count; i++)
                {
                    if (!_words[i].Activated)     //如果这个单词已经不会再被背诵，则不处理它
                        continue;
                    if (_words[i].Bucket < currentBucketId)
                        currentBucketId = _words[i].Bucket;

                    if (_words[i].Bucket > maxBucket)//如果Bucket不够多，添加新的Bucket
                    {
                        for (int j = maxBucket + 1; j <= _words[i].Bucket; j++)
                            _buckets.Add(new Bucket());
                        maxBucket = _words[i].Bucket;
                    }

                    if (_words[i].isNewWord())//判断是否是旧单词，从而决定将其放入旧单词表还是新单词表
                        _buckets[_words[i].Bucket]._newWordList.Add(_words[i]);
                    else
                        _buckets[_words[i].Bucket]._oldWordList.Add(_words[i]);
                }

　　

这段代码则没有注释：

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using DictLoaderTest;
using System.Windows;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Xml.Serialization;
using SimpleEnglishLearning;

public class CourseCollection : KeyedCollection<string,CourseBasicInfo>, INotifyCollectionChanged, INotifyPropertyChanged
{
    public User User { get; internal set; }

    public ICourse CurrentCourse { get; protected set; }

    private int currentindex;
    public bool LoadCourse(int index)
    {
       //-----------------------------------------------------------------------------------
       //中间省去200行
       // ------------------------------------------------------------------------------------
    public WordInfoCollection(IWordDictionary book,WordOrder order, int listsize)
    {
        int i=0,listid=1;
        string[] words = book.Dictionary.Keys.ToArray();
        WordAlgo.SortWords(ref words, order);
        foreach (string word in book.Dictionary.Keys)
        {
            if (i >= listsize)
            {
                listid++;
                i = 0;
            }
            WordInfo wi = new WordInfo(word);
            wi.ListID = listid;
            this.Add(wi);
        }
    }
}

　　

2.代码出现不可知数字：

theshold = 30;

const int THRESHOLDCONST = 30;
threshold = THRESHOLDCONST;

　　通过注释知道这个theshold是两个单词相似度的阀值（threshold），这处应该是拼写错误。第一种实现方法，之后会多次出现30这个数字，作为一个项目外的人员，会多次想为什么这里出现要出现一个30，而且以后需要修改这个数字，不得不找出全部的30，然后改成29（譬如）。第二种实现方法则成功解决以上两个问题。所以在代码中不出现不可知数字和不可知字符或字符串，还是非常有必要的！

 

3、冗余代码高：

if (currentBucket.isOldWordListEmpty())  //当前bucket _oldWordList为空的情况
            {
                currentWord = currentBucket._newWordList[0];
                currentBucket._oldWordList.Add(currentWord);
                currentBucket._newWordList.Remove(currentWord);
                return currentWord;
            }
            else if (currentBucket.isNewWordListEmpty()) // 当前bucket _newWordList为空的情况
            {
                currentWord = currentBucket._oldWordList[ran.Next(currentBucket._oldWordList.Count)];
                return currentWord;
            }
            else //均不为空的情况下，按公式计算概率，选取旧单词或新单词
            {
                if (currentBucket._oldWordList.Count <= currentBucket._newWordList.Count)
                {
                    if (ran.NextDouble() > Math.Pow(1.0 * currentBucket._oldWordList.Count / currentBucket._newWordList.Count, 0.25) / 2)
                    {
                        currentWord = currentBucket._newWordList[0];
                        currentBucket._oldWordList.Add(currentWord);
                        currentBucket._newWordList.Remove(currentWord);
                        return currentWord;
                    }
                    else
                    {
                        currentWord = currentBucket._oldWordList[ran.Next(currentBucket._oldWordList.Count)];
                        return currentWord;
                    }
                }
                else
                {
                    if (ran.NextDouble() <= Math.Pow(1.0 * currentBucket._newWordList.Count / currentBucket._oldWordList.Count, 0.25) / 2)
                    {
                        currentWord = currentBucket._newWordList[0];
                        currentBucket._oldWordList.Add(currentWord);
                        currentBucket._newWordList.Remove(currentWord);
                        return currentWord;
                    }
                    else
                    {
                        currentWord = currentBucket._oldWordList[ran.Next(currentBucket._oldWordList.Count)];
                        return currentWord;
                    }
                }

这段代码中的大部分都属于冗余代码，出现冗余代码第一增加了工作量（好吧你是复制粘贴的），第二增加了发成错误的概率，第三增加了维护成本（三倍工作量）。将这段代码抽象成一个函数这个不错的选择！

 

总体来说SuperBrother的代码规范还是很好的，就是有些代码太高端了。