JdonFramework代码探讨

粗略读了读该架构的代码,感觉总体写的还是不错的。
提出了些建议,仅供参考。

其实我们平常在做项目时,当要进入编码阶段时,
首先应该指定编码的规范,然后大家按照这个规范进行编码。
如果不制定一个严格的编码规范,大家随意发挥,会给将来
的维护带来很大的麻烦,相信大家也都有这个体会。

该架够很大的问题就是在这方面没有太注意,好多类中通篇
连一点注释都没有,让我们这些开源爱好者学习起来太费劲了。
有的虽有些注释,但注释的风格花样叠出,一个优良的系统如
果从编码的角度看应该尽量让人觉得这个系统是一个人编出来的,
风格非常统一,代码质量高,而且漂亮。
还有提个建议,既然是开源的,大家不妨使用Eclipse作为开发工具,
都开源吗?使用Jbuilder也行,可要小心哪天别撞到枪口上哟。

以下是些具体的点,仅供参考。

1.PropsUtil.java的问题点

(1)变数定义后未使用
public final static String SETUPNAME = "setup";
public final static String SETUPVALUE = "true";

(2)loadProperties方法
52-56和58-63段代码不知有啥区别。


2.HttpSessionProxyVisitor.java的问题点
(1)removeEJBObject方法
下面代码的112行有必要吗?
程序执行到112行时ccEjb肯定不是null的,如果从测试的角度看,112行这段代码至少需要测试两个点
一是:ccEjb != null
二是:ccEjb == null
所以这个判断可一去掉的吧。
111: if (ccEjb instanceof EJBLocalObject) {
112: if (ccEjb != null) {
EJBLocalObject eo = ( (EJBLocalObject) ccEjb);
eo.remove();
}
} else if (ccEjb instanceof EJBObject) {
if (ccEjb != null) {
EJBObject eo = ( (EJBObject) ccEjb);
eo.remove();
}
121: }


3.FileUtil.java的问题点
(1)recursiveRemoveDir方法
①205行:filelist = null; 不知有啥特殊意思,感觉挺怪怪的

②188行: if (tmpFile.isDirectory()) {
190行: } else if (tmpFile.isFile()) {
这两个判断条件有点那个了,既然不是目录,就是文件了,这样写虽说没错,但。。。
188行: if (tmpFile.isDirectory()) {
190行: } else {
改成上述语句如何,仅供参考。

(2)CopyDir方法
①方法名用大写,不太好吧。

②参数名sourcedir,destdir如果该成sourceDir,destDir是不是更好点,
JAVA中一般变数第一个字母是小写,后面的每个单词的第一个字母一般都是大写,这样符合大多数人的习惯吧。

(3)该类中一个最致命的错误就是好多数据流的关闭处理都不太严紧,有可能会成为瓶颈的。
例如:readFile方法
假如在该行
while ( (len = br.read(buffer)) > -1) {
读取数据时发生错误了,那么就导致以下两行代码
br.close();
fr.close();
是无效的,br,fr数据流都没有被关闭,这些都是隐患的哟。

(4)readFile方法的函数说明
该函数只有一个参数input,而函数说明中确有output,content两个参数说明。

(5)copy方法
在该方法中定义了变数BUFSIZE,是大写,
一般常量都是在类中用大写变量来定义的,
上述用法应该说是很少见的。
int BUFSIZE = 65536;


4.MultiHashMap.java
在JAVA的编码规约中,该空的空一个格,效果会很好的。请看以下的地方。
(1)put方法
①空格的应用
25行:public Object put(Object key,Object subKey,Object value){
修正:public Object put(Object key, Object subKey, Object value){

27行:if(a==null){
修正:if (a == null) {

29行:super.put(key,a);
修正:super.put(key, a);

虽然对程序的执行没什么影响,但适当的加空格,代码会变得易读和漂亮。

②变量名的定义
如该行:HashMap a = (HashMap)super.get(key);
变量名是a,变量名的使用一般让他人在读你的代码时,
能光看变量名就知道该变量想表达什么意思。
变量名a,感觉不太专业。


5.VisitorFactory.java的问题点
26行:
private final static String module = VisitorFactory.class.getName();
相当于: private final static String module = “VisitorFactory”;
在38行:
if (cmKey == null){
做了个条件判断,其实这个判断条件能走进去吗???


6.LoginServlet.java的问题点
41行: public final static String Login_Module = "SecurityRealm";
42行: private String Login_Module_Name;
43行: private String Login_Home_Uri;
该三行的变量名第一个字母用大写,不太符合JAVA编程的习惯吧。
JAVA中常量定义一般都是大写字母,变量名首字母是小写,后面每个单词的首字母是大写。


7.ImageShowAction.java的问题点
下面这个函数的数据流的关闭处理采用这种方法不太好吧。要是在finally块中执行
toClient.close();
出错,则数据流没被关闭,其实一般的做法是在写完数据后就关闭,然后在finally
块中判断数据流有无关闭,没关闭则在执行一次关闭较安全些。
private void outImage(HttpServletResponse response, byte[] data) throws
Exception {
response.setContentType("images/jpeg");
OutputStream toClient = response.getOutputStream();
try {

toClient.write(data);

} catch (Exception ex) {
Debug.logError("get the image error:" + ex, module);
} finally {
toClient.close();
}

}


8.ModelListForm.java和EventModel.java
如下两段代码,都是get方法返回一个变数,可
如 return (this.allCount);
这种用法太少见了,出现截然不同的两种写法,
是不是很怪啊,虽然都没错,但从代码审美的角度来看,有点不美了吧。
public int getAllCount() {
return (this.allCount);
}

public Model getModel() {
return this.model;
}
建议只采用第二种方式。

9.关于版权的问题点
例如:
InvokerServlet.java
47行: * <p>Copyright: Jdon.com Copyright (c) 2003</p>

WebAppUtil.java
33行: * <p>Copyright: Jdon.com Copyright (c) 2005</p>
一套系统的版权怎么能既是2003的又是2005的呢。

多谢bluebears仔细阅读。


由于这个框架应用在很多应用系统中,几经更改,所以平时不敢贸然删除整理一些看上去是多余的东西。
com.jdon.util下大部分代码是借用的OFBiz等开源系统,已经申明是它们的版权。

1. PropsUtil.java的 SETUPNAME 可能已经废弃,待确证。
2. HttpSessionProxyVisitor.java 是测试时加入,待确证
3. FileUtil.java是借鉴其它系统,编写是有问题,会改进的。
4. MultiHashMap.java也是借鉴其它系统。会改进的。
5. VisitorFactory.java这个有bug,纠正。
6. LoginServlet.java这个几个书写特别,是引起重视,有些特殊应用场景在后面。
7. ImageShowAction.java这种关闭方式一般情况就可以了,如果需要更安全,为关闭写一个专门方法即可。
8. ModelListForm.java和EventModel.java写法应该统一。
9.关于版权怎么能既是2003的又是2005的,有的是在2003开发的,这方面细节会注意,最好统一。

jdonFramework非常缺少您这样细心的人,如果有意愿,有时间和精力,有热情,一起加入,可给我来信。