代码复审统计
FileOperate && DownLoadFile——张永强
我主要负责FileOperate和DownLoadFile两个类的复审工作:
下面是我复审过程中的一些问题:
首先是FileOpreate类:
因为没有100coding组的代码规格和要求说明,所以只能够以我的理解来说了。其中的一些不一定对,可能更多的是符合我个人的习惯。
总体来说我觉得他们的代码还是比较整洁的。整个代码的格式都是比价规范的。下面只是挑一些我个人的疑惑。
1、 没有关于整个类的作用的说明。
2、 代码中的命名还是比较规范的,方法和变量名称均采用了Camel命名方法,这样也可能造成方法和变量的区分不是很明显。也有个别地方命名不好
这里的fs命名就不是很好。
3、 没有注释,或者说是有没有意义的注释。这个类中出现的注释只是下面的两种情况:
4、 程序中存在没有实现的空方法
5、 下面的这个方法我没有看明白,为什么还要使用一次filePath.toString()
6、 关于整形数的使用问题,我觉得还是特别申明一个常量好(这一点没有表述清楚)
比如说上面的1444,我觉得可以声明一个常量。
7、 关于流的使用
这里最后只对inStream进行了处理,而没有对fs进行处理
8、 关于{}的问题
程序中全部是使用这种格式的大括号。这个与具体的使用习惯有关。
9、 还有一问题,就是前面说到了没有关于整个类的功能的叙述,所以只是依靠我自己的理解,我发现其中有File和Folder,不知道这个类是专门操作文件还是文件夹或者两者皆有,但是总的来说的话,我觉得还是应该要注意这一点的。如果是两者都有的话可能还差一些方法。
DownLoadFile类:
这个类比较简单,只包含了4个方法。分别是DownLoadFile(); run(); getFileNameByUrl(); saveToLocal();
同样,这个种的命名还是比较规范的,这个类相对前面的类的注释有了很好的提高。可能是觉得前一个类太简单了,不用加注释了。
下面仍然简单地列出其中一些我个人觉得的问题:
1、 注释不太准确
这个注释放在了类内部的最前面的地方。我就理解为是介绍这个类功能的注释了,但是显然这个不是很准确。从类名应该知道主要是下载文件之类的作用吧。
2、 关于变量的声明
这个是直接在声明的时候new操作了,个人觉得还是最好放到构造函数中。
3、 关于public的变量,还有关于这里的访问权限的控制,我自己习惯是用private
我没有看count具体是什么定义,但是最好能够不要设置public访问权限。
4、 同样仍然是关于常量的使用问题
5、 异常的捕捉
这里注释说的网络异常我不是很理解。
6、 网页类型
这里对于url的处理时寻找.html后缀,首先,可能有的网页时以.htm后缀结尾的,也可能是一些其他的后缀,判断可能对于爬取的结果造成的影响。
同时,对于url的处理,在课堂上已经说过了,这里不再多说。
7、 文件类型的处理
同样,通过这里,我们也能够看到处理不同类型的文件的时候考虑的不完善。只是ppt,doc,但是pptx,docx这些都没有考虑到。
不过总体来说,这个类写得还是非常不错的。从代码的规范角度上来说。但是从代码的功能角度上,考虑可能还有所欠缺。
得分:80分;
URL && Quene && DenseEdglist——王伟东
我主要负责Url、Queue和DenseEdglist这3个类的代码复审
第一个Url是一个封装的用于存储url信息的类,没有很特别的地方,仅仅是一些属性和其对影的get和set的方法
第二个Queue类是对一个LinedList这个类进行了一种包装吧
我不清楚这样的格式对于程序的可读性和维护的难易度是否有影响。不过我觉得这个类可以不要···
第三个类DenseEdgeList,我写一些我的看法
1.文档不完善
这个代码非常明显,使用了一个固定大小的256的数组,而且没有给一个文档对此进行说明,很可能在后续开发中造成数组越界等情况
2.变量等命名很规范
从这2个例子可以看出,是比较规范的,提高了代码的可读性
3.有些地方我认为不妨简化表达式
比如这个方括号中进行的运算是一个位与运算,比较复杂,可以增加适当的注释
总评:85分
这个代码都很简单,是一些信息类和一些实现了几个简单功能的方法
变量命名规范,其方法设定也很合理
不过代码由于缺失一些注释,很可能造成一些潜在的威胁
htmlParserTool && LinkFilter && MyCrawler——王莹
一.代码风格:该组同学在代码风格方面做得还不错。首先以命名规范来讲,遵守Java语言的命名规范,包括包名,类名,方法名等。但有些变量定义的标示符命名过于随意,比如在MyCrawler.java这个文件中,在展示界面的部分对于一些JLabel,JButton的命名过于随意。
比如:
JLabel jl2=new JLabel("已访问URL个数 : " + 0); JLabel jl00=new JLabel("要下载网页个数 :"); JTextField jtf=new JTextField(10); JButton jb1=new JButton("打开"); JButton jb2=new JButton("关闭"); JPanel jp=new JPanel(); JProgressBar jpb=new JProgressBar(); public CraUi() { this.setTitle("界面"); jp.setLayout(null); jl00.setBounds(30, 20, 280, 30); jp.add(jl00); jtf.setBounds(150, 20, 280, 30); jp.add(jtf); jl2.setBounds(30, 130, 280, 30); jp.add(jl2); jb1.setBounds(50, 180, 80, 30); jp.add(jb1); jb1.addActionListener(this); jb2.setBounds(250, 180, 80, 30); jp.add(jb2); jb2.addActionListener(this); jpb.setBounds(30, 300, 500, 50); jpb.setStringPainted(true); jpb.setMinimum(0); jpb.setValue(0); jpb.setIndeterminate(true); jp.add(jpb); this.add(jp); this.setBounds(300, 250, 600, 500); this.setVisible(true); }
上面对于对于标签和按钮的命名没有实际的意义。从而导致我们无法直接根据变量名判断该控件的作用。
二.代码注释
1.按着书上的说法,注释(包括源代码)应该只用ASCII字符,不要用中文或其他特殊字符,这样会极大的影响程序的可移植性。而本组同学的所有注释全部是中文,显然不符合要求。
2.注释是因该随着程序的修改不断更新的。而如果不是这样的话,一个误导的注释可能比没有注释更加糟糕。
比如:
//定义过滤器,提取以http://www.lietu.com开头的链接 LinkFilter filter = new LinkFilter(){ public boolean accept(String url) { int flag=0; if(url.startsWith("http:")) flag=1; else flag=1; if(url.indexOf("html")!=-1||url.indexOf("ppt")!=-1||url.indexOf("pdf")!=-1||url.indexOf("doc")!=-1) flag=1; else { flag=0; } if(flag==0) return false; else return true; } };
在上面的代码中,开头的注释告诉我们是提取以这个URL开头的链接,但程序中显然不是这样的。不知道是同学从网上Copy代码的时候忘了销毁犯罪记录了还是怎么回事。对于这个简单的程序可能这么一句错误的注释没什么,可是如果对于一些关键的算法部分出现这样的错误可能会导致整体理解的错误。
3.注释表达不严谨
//循环条件:待抓取的链接不空 number=0; while(number<Cr.Tint) { //队头URL出队列 if(LinkQueue.unVisitedUrlsEmpty()||count>=20 ) { continue; } String visitUrl=(String)LinkQueue.unVisitedUrlDeQueue(); if(visitUrl==null) continue; DownLoadFile downLoader=new DownLoadFile(visitUrl,filter); //下载网页 downLoader.start(); number++; System.out.println("number:"+number); }
这里的循环条件是存储URL的队列是否为空,而不是链接是否为空。
三.程序功能
在eclipse上运行该组同学的程序,能够正常运行。但是阅读一下源代码发现还是有一些问题。
1. 在下载文档的时候,只能够下载后缀为pdf、doc、ppt的文档。而对于pptx、docx等类型的文档下载则不支持。
2. 有些代码过于冗余。
public boolean accept(String url) { int flag=0; if(url.startsWith("http:")) flag=1; else flag=1; if(url.indexOf("html")!=-1||url.indexOf("ppt")!=-1||url.indexOf("pdf")!=-1||url.indexOf("doc")!=-1) flag=1; else { flag=0; } if(flag==0) return false; else return true; }
比如上面的代码,为了实现相同的目的,完全可以写成这样
public boolean accept(String url) {
if(url.indexOf("html")!=-1||url.indexOf("ppt")!=-1||url.indexOf("pdf")!=-1||url.indexOf("doc")!=-1)
return true;
return false;
}
这里有一点要说明,我是就着上面的代码这样改的。而实际上可能是这个同学在写代码的时候写错了。对于上面的红色部分,可能本意是else falg=0;
四.得分:总体来讲,这组同学做的还是很好的。那就打个85分吧。
AhoCorasick && AcApply——刘昕
AhoCorasick.java
有详细的注释与说明,细心的添加了更新信息,还有比较详细的使用方法说明。
唯有的一点下次在变量的命名上略有不足:
譬如
r = state;
byte a = keys[i];
State s = r.get(a);
一类的语句语义比较模糊,虽然有注释但是还是希望能在变量名的命名上更细致一点
85分
AcApply.java
代码中规中矩,框架清晰,变量命名十分的有条理,
但是通篇只有三个注释,例如这里:
在这其后根据异常类型抛出相应的异常,代码写得十分清晰,但是前面这一段具体是做什么的还需要进行注释解释。
否则不利于以后的代码维护工作。80分
posted on 2012-12-11 22:23 fightingsnail1 阅读(350) 评论(0) 编辑 收藏 举报