让人头疼的新手

刚进公司没多久时，领导让我带两个新人(07年7月份毕业的)。他们两个是试用期3个月都过了之后才参与到我们现在的项目中来的，算起来他们也工作了快一年了，可是今天无意中修改一个他们写过的一个类文件时，看到他们写的一些代码，让我又好气又好笑。可能也是我以前只注意给他们测试功能了，没怎么注意代码方面的问题吧！

晒晒他们的部分代码！

/**
	 * 查询方案金额总和
	 * 
	 * @param lottype
	 *            Long
	 * @return double
	 */
	public Double getsummation(Long lottype)
	{
		double sum = 0.00;
		List ls = null;
		String sql = "";
		if (sql != null)
		{
			sql = " select sum(amount) from OccurProj a where a.lottype = '" + lottype + "' ";
		}
		ls = find(sql);
		if (ls != null && ls.isEmpty())
		{
			for (int i = 0; i < ls.size(); i++)
			{
				OccurProj occurproj = (OccurProj) ls.get(i);
				sum = sum + occurproj.getAmount();
			}
		}
		return sum;
	}

/**
	 * 根据主键查询明细信息
	 * 
	 * @param occurproj
	 *            OccurProj
	 * @return String
	 */
	public List queryById(Long id)
	{
		String sql = "";
		if (sql != null)
		{
			sql = " from OccurProj a where a.id=" + id;
		}
		return translateCode2Str(find(sql));
	}

/**
	 * 参数查询
	 * 
	 * @param occurproj
	 *            OccurProj
	 * @param lottype
	 *            Long
	 * @return List
	 */
	public List queryByPara(String username, Long lottype)
	{
		String sql = "";
		if (username != null && username.length() > 0)
		{
			sql += " and a.username like ? ";
		}
		if (sql.length() > 1)
		{
			sql = " from OccurProj a where 1=1 and a.lottype = '" + lottype + "' " + sql;
		} else
			sql = " from OccurProj where lottype = '" + lottype + "' ";
		return translateCode2Str(find(sql));
	}

我现在也体会到当初我找工作的时候为什么那么难了，同时也提醒一下即将毕业的朋友，想找到不错的工作，你的工作态度确实得端正！

以前没带过新人，大家也介绍一下，带新人的经验吧！


我需要说明一下，我发贴的目的是想提醒一下临近毕业的朋友，不是公司不需要应届毕业生，应届毕业生有优点也有缺点。但是你进入公司以后，自己的发展和进步还是要靠自己的努力。工作了半年还能写出这样令人佩服的代码，那就只能是你自己的问题了，当然也有我自己的责任，毕竟是我带他们。

有则改之，无则加勉，别无他意，仅此而已！

评论

39 楼 mylifestyle1225 2008-05-13  

colin4k 写道

mylifestyle1225 写道

colin4k 写道

引用

if(a_binary.substring(index,index+1).equals("0")){

如果a_binary为null怎么办？
如果a_binary.substring(index,index+1)为null怎么办？
代码里面类似的缺陷很多

谢谢你能仔细看我的代码及你的意见.
但是a_binary为null是不可能的.
int i=a|256;
String is=Integer.toString(i, 2);
a是一个byte,a|256,所以is的长度肯定是9
又因为只需在for(int i=0;i<8;i++)的条件下调用该方法,所以index只能取0-7,因此a_binary.substring(index,index+1)也不可能为null.
如果逻辑有错,希望各位纠正下..
如果还是觉得可能为null,那就举出反例吧.

一个团队合作的项目，你如何保证别人在调用这个函数时，只传0-7的index进来？就算用注释来说明都是不足够的，
如果你希望它的取值范围只能是0-7也只会是0-7，那你应该用代码来保证，例如在函数一开始就验证其取值的正确性
这种陷阱是项目里面非常常见的，也是区分新手和有经验的开发者的重要方法。

团队合作的项目,是否意味着你写的每个方法别人都可能去调用?或者说你每写一个方法,都需要考虑到别人调用??
你在接口里有N个方法,接口的实现类里也只能是N个方法吗?难道不会抽象出来几个方法供该实现类自己使用?
注意,我这个方法只服务于其所在的这个实现类,并没有想要让别人调用.我希望别人调用的方法,都在了这个类所实现的接口里.
我希望取值范围只能是0-7,所以for(int i=0;i<8;i++)中的i不是保证了吗?这个方法只用于两个for(int i=0;i<8;i++)下,我没必要连着写两遍,于是我抽象出来了这个方法,这就是这个方法的由来.
参数int index代表byte　a中第index个bit,自然取(x,x+7)了,一个byte还能有几个bit啊.
我考虑问题不会考虑得那么片.虽然只是个新人,但肯定会把所有的可能都考虑进去.这个是习惯.

38 楼 yuan 2008-05-13  

我最近在读类似的代码，读完了要理解流程，然后参与到开发中。可怜的我。。

37 楼 colin4k 2008-05-13  

mylifestyle1225 写道

colin4k 写道

引用

if(a_binary.substring(index,index+1).equals("0")){

如果a_binary为null怎么办？
如果a_binary.substring(index,index+1)为null怎么办？
代码里面类似的缺陷很多

谢谢你能仔细看我的代码及你的意见.
但是a_binary为null是不可能的.
int i=a|256;
String is=Integer.toString(i, 2);
a是一个byte,a|256,所以is的长度肯定是9
又因为只需在for(int i=0;i<8;i++)的条件下调用该方法,所以index只能取0-7,因此a_binary.substring(index,index+1)也不可能为null.
如果逻辑有错,希望各位纠正下..
如果还是觉得可能为null,那就举出反例吧.

一个团队合作的项目，你如何保证别人在调用这个函数时，只传0-7的index进来？就算用注释来说明都是不足够的，
如果你希望它的取值范围只能是0-7也只会是0-7，那你应该用代码来保证，例如在函数一开始就验证其取值的正确性
这种陷阱是项目里面非常常见的，也是区分新手和有经验的开发者的重要方法。

36 楼 mylifestyle1225 2008-05-13  

colin4k 写道

引用

if(a_binary.substring(index,index+1).equals("0")){

如果a_binary为null怎么办？
如果a_binary.substring(index,index+1)为null怎么办？
代码里面类似的缺陷很多

谢谢你能仔细看我的代码及你的意见.
但是a_binary为null是不可能的.
int i=a|256;
String is=Integer.toString(i, 2);
a是一个byte,a|256,所以is的长度肯定是9
又因为只需在for(int i=0;i<8;i++)的条件下调用该方法,所以index只能取0-7,因此a_binary.substring(index,index+1)也不可能为null.
如果逻辑有错,希望各位纠正下..
如果还是觉得可能为null,那就举出反例吧.

35 楼 mars李 2008-05-13  

我感觉有点像刻意写黑色笑话的．．．
逻辑非常混乱．．不是一般人能这么琢磨出来的

34 楼 rockjava 2008-05-13  

恩，这代码写的有点太....

33 楼 林杰杰 2008-05-13  

　　假如在我们公司用英语注释，那是一种很不靠谱的行为。。。
　　首先，本身的英语水平就不高，起个变量名还得左思右想。用英语注释就更加辞不达意了，还得把大部分精力花在注释上。

　　其次，注释的人水平很高，但是读注释的人水平很次，那时候注释反而成为了一种障碍。

　　最后，你还嫌英语信息不够多是不？

32 楼 armorking 2008-05-13  

colin4k 写道

英文注释是为了确保不会因为encode等原因导致乱码

这种说法就好比是“为了防止脚气就把腿锯掉”

31 楼 daoger 2008-05-13  

谢谢LS各位的回复！我需要说明一下，我发贴的目的是想提醒一下临近毕业的朋友，不是公司不需要应届毕业生，应届毕业生有优点也有缺点。但是你进入公司以后，自己的发展和进步还是要靠自己的努力。工作了半年还能写出这样令人佩服的代码，那就只能是你自己的问题了，当然也有我自己的责任，毕竟是我带他们。

有则改之，无则加勉，别无他意，仅此而已！

30 楼 colin4k 2008-05-13  

引用

if(a_binary.substring(index,index+1).equals("0")){

如果a_binary为null怎么办？
如果a_binary.substring(index,index+1)为null怎么办？
代码里面类似的缺陷很多

29 楼 colin4k 2008-05-13  

英文注释是为了确保不会因为encode等原因导致乱码

28 楼 photon 2008-05-13  

一些应届生的问题都看见了，怎么解决呢？
不招应届生，或者找符合条件的来用。但找不到有经验的或者符合条件的呢？

27 楼 mylifestyle1225 2008-05-13  

if else中的确该优化下,赶时间了.我已经把相同的抽象出来了.谢谢指点了.不过LS也来优化一下吧,然后把代码贴出来,指导下新人.别指说不做,牛人不是用嘴吹的.

26 楼 deafdragon 2008-05-13  

colin4k 写道

mylifestyle1225 写道

String sql = "";  
        if (sql != null)
哈哈哈哈哈.....
我也07年7月份毕业,工作也就3个月,但也不至于这么糊涂.另外,小晒一下刚写完代码.


public Map<Integer, String> getFields(String message) throws UnsupportedEncodingException {
// TODO Auto-generated method stub
BitSet bitMap=new BitSet();
boolean isExtend=false;
Map<Integer,String> fields=new HashMap<Integer,String>();//用来存放报文标识类型（key=0），位图（key=1），2-128个域（key=2--key=128）
byte[] all = message.getBytes("ISO-8859-1");//把字符串转成byte[]
byte[] messageType=new byte[4];//存放报文类型标识符的byte[]
for(int i=0;i<4;i++){
messageType[i]=all[i];
}
fields.put(0, new String(messageType,"ISO-8859-1"));//放入Map<Integer,String> fields
if(all[4]<0){//如果有扩展位图
isExtend=true;
}
//初始化基本位图,根据检索基本位图的8个字节的二进制的结果(第4-11字节)，把bitMap的32位到95位设置成true（1）或者false（0），
int count=4;
while(count<12&&count>3){
byte each=all[count];
count++;
for(int i=0;i<8;i++){
if(getValue(each,i)){//如果返回true，即二进制上是1，把该bit位设置成true，默认为false
bitMap.set((count-1)*8+i,true);
}
}
}
//如果有扩展位图,初始化扩展位图,根据检索扩展位图的8个字节的二进制的结果(第12-19字节)，bitMap的96-153,设置成true(1)或者false（0）
if(isExtend){
while(count>11&&count<20){
byte each=all[count];
count++;
for(int i=0;i<8;i++){
if(getValue(each,i)){
bitMap.set((count-1)*8+i,true);//如果返回true，即二进制上是1，把该bit位设置成true，默认为false
}
}
}
}
/*测试位图
  for(int z=32;z<bitMap.size();z++){
if(bitMap.get(z)){
System.out.println(z);
}
}*/
System.out.println("位图初始化完成");
//初始化位图完成,开始读取数据，i=1表示从bitMap的第2个位置开始循环读取。
if(isExtend){
int currentByte=20;
for(int i=32;i<161;i++){
currentByte=putFields(fields,currentByte,bitMap,i,all);
fields.put(1, "1");
}
}else{
int currentByte=12;
for(int i=32;i<161;i++){
currentByte=putFields(fields,currentByte,bitMap,i,all);
fields.put(1, "0");
}
}
return fields;
}

//检索一个byte二进制中对应位置上的值的方法，返回false表示"0",true表示"1"; 参数int index取0-7,a取[-128到127]
public boolean getValue(byte a,int index){
//把一个byte转换成类似"01000000"的形式,并用一个String的值表示
String a_binary;
if(a>-1&&a<128){
int i=a|256;
String is=Integer.toString(i, 2);
a_binary=is.substring(1, 9);
}else{
int i=(-a)|256;
String is=Integer.toString(i, 2);
a_binary="1"+is.substring(2, 9);
}
if(a_binary.substring(index,index+1).equals("0")){
return false;
}else{
return true;
}
}

用我们公司的代码标准来批判一下这份代码：
1、函数前没有符合JavaDoc标准的注释，-5分；
2、函数名不知道要表达什么意思，-5分；
3、注释是中文而不是英文，-2分；
。。。。。。

magic number竟然可以这么滥用... ...中文注释... .... 一个几十行的函数。
if else不同的分支里执行的代码有相同的内容。

说你代码滥，冤枉你了？

25 楼 mylifestyle1225 2008-05-13  

armorking 写道

mylifestyle1225 写道

不过还是谢谢指点,我会改成英文的.然后把名字改得尽量意义明确些.

如果不是对美外包的项目的话，为什么非要用英语写注释

恩，是不是对美外包的,这是个金融报文的解析和生成,所以自然没什么SQL,JDBC,SPRING等东东了.
有道理,注释的作用本来就是为了让人看懂.

24 楼 armorking 2008-05-13  

mylifestyle1225 写道

不过还是谢谢指点,我会改成英文的.然后把名字改得尽量意义明确些.

如果不是对美外包的项目的话，为什么非要用英语写注释

23 楼 mylifestyle1225 2008-05-13  

不过还是谢谢指点,我会改成英文的.然后把名字改得尽量意义明确些.

22 楼 mylifestyle1225 2008-05-13  

注释我写了...没复制过来而已.你再给我+5分吧.
第2,3条我没异议,扣吧..
纠正下，我刚写完，还没上交了.但测试完了.

21 楼 colin4k 2008-05-13  

mylifestyle1225 写道

String sql = "";  
        if (sql != null)
哈哈哈哈哈.....
我也07年7月份毕业,工作也就3个月,但也不至于这么糊涂.另外,小晒一下刚写完代码.


public Map<Integer, String> getFields(String message) throws UnsupportedEncodingException {
// TODO Auto-generated method stub
BitSet bitMap=new BitSet();
boolean isExtend=false;
Map<Integer,String> fields=new HashMap<Integer,String>();//用来存放报文标识类型（key=0），位图（key=1），2-128个域（key=2--key=128）
byte[] all = message.getBytes("ISO-8859-1");//把字符串转成byte[]
byte[] messageType=new byte[4];//存放报文类型标识符的byte[]
for(int i=0;i<4;i++){
messageType[i]=all[i];
}
fields.put(0, new String(messageType,"ISO-8859-1"));//放入Map<Integer,String> fields
if(all[4]<0){//如果有扩展位图
isExtend=true;
}
//初始化基本位图,根据检索基本位图的8个字节的二进制的结果(第4-11字节)，把bitMap的32位到95位设置成true（1）或者false（0），
int count=4;
while(count<12&&count>3){
byte each=all[count];
count++;
for(int i=0;i<8;i++){
if(getValue(each,i)){//如果返回true，即二进制上是1，把该bit位设置成true，默认为false
bitMap.set((count-1)*8+i,true);
}
}
}
//如果有扩展位图,初始化扩展位图,根据检索扩展位图的8个字节的二进制的结果(第12-19字节)，bitMap的96-153,设置成true(1)或者false（0）
if(isExtend){
while(count>11&&count<20){
byte each=all[count];
count++;
for(int i=0;i<8;i++){
if(getValue(each,i)){
bitMap.set((count-1)*8+i,true);//如果返回true，即二进制上是1，把该bit位设置成true，默认为false
}
}
}
}
/*测试位图
  for(int z=32;z<bitMap.size();z++){
if(bitMap.get(z)){
System.out.println(z);
}
}*/
System.out.println("位图初始化完成");
//初始化位图完成,开始读取数据，i=1表示从bitMap的第2个位置开始循环读取。
if(isExtend){
int currentByte=20;
for(int i=32;i<161;i++){
currentByte=putFields(fields,currentByte,bitMap,i,all);
fields.put(1, "1");
}
}else{
int currentByte=12;
for(int i=32;i<161;i++){
currentByte=putFields(fields,currentByte,bitMap,i,all);
fields.put(1, "0");
}
}
return fields;
}

//检索一个byte二进制中对应位置上的值的方法，返回false表示"0",true表示"1"; 参数int index取0-7,a取[-128到127]
public boolean getValue(byte a,int index){
//把一个byte转换成类似"01000000"的形式,并用一个String的值表示
String a_binary;
if(a>-1&&a<128){
int i=a|256;
String is=Integer.toString(i, 2);
a_binary=is.substring(1, 9);
}else{
int i=(-a)|256;
String is=Integer.toString(i, 2);
a_binary="1"+is.substring(2, 9);
}
if(a_binary.substring(index,index+1).equals("0")){
return false;
}else{
return true;
}
}

用我们公司的代码标准来批判一下这份代码：
1、函数前没有符合JavaDoc标准的注释，-5分；
2、函数名不知道要表达什么意思，-5分；
3、注释是中文而不是英文，-2分；
。。。。。。

20 楼 mylifestyle1225 2008-05-13  

敢SHOW就敢挨砖头,这些只是解析一个报文的局部,不想占用太大篇幅,只是对字符串及数组,二进制的操作.非得涉及什么SQL,JDBC或者某某opensource你才觉得是高深?那些也未必不是不会呀.
我也就工作三个月,写了一个月代码.随便你们怎么说了,但至少没那么可笑的错误.
我只想说明应届生不是都像LZ说的那样罢了,让我去写LZ的程序,我不会出这样的错.