我一辈子见过最糟糕的代码
翻译的很烂,具体请看原文: http://blog.cherouvim.com/the-worst-codebase-ive-seen-in-my-life/
最近,我接了一个庞大的老项目(我们称之为FailApp),其中包括很多不合格的代码。 我们将其分为三类。
1) LoLs: 低级笑话代码(LOL是文字聊天里的大笑表情)
2) WTFs: 他妈的烂代码(what the fuck)
3)ShowStopper: 超级危险代码,致命的,制造P1级事故的代码!
直接举例:
LoL#1
在应用程序上下文中使用“Prod”这个的名称。
导致后面的这个测试环境http://123.12.34.123:7001/FailAppProd看起来像生产环境
LoL#2
EscapeIlegalWords eiw = new EscapeIlegalWords();
foo = eiw.escapeIlegalWords(foo);
public static String escapeIlegalWords(String code) {
code = code.replaceAll("<", "<");
code = code.replaceAll(">", ">");
code = code.replaceAll("&", "&");
code = code.replaceAll("'", "'");
code = code.replaceAll(""", "\"");
code = code.replaceAll(" ", "");
return code;
}
- 我的妈呀,这是调用静态方法的新的实例吗?
- HTML过滤,还是非法字符过滤?
- 过滤是单向的吗?
- 起这样的类名字有意义吗?
- 这样的算法,性能如何?
LoL#3
session.setAttribute("contentXML", null);
session.removeAttribute("contentXML");
session.setAttribute("contentOwnerList", null);
session.removeAttribute("contentOwnerList");
session.setAttribute("structuresVector", null);
session.removeAttribute("structuresVector");
session.setAttribute("selectedThematic", null);
session.removeAttribute("selectedThematic");
session.setAttribute("selectedTarget", null);
session.removeAttribute("selectedTarget");
session.setAttribute("targetList", null);
session.removeAttribute("targetList");
session.setAttribute("thematicList", null);
session.removeAttribute("thematicList");
session.setAttribute("metadatas", null);
session.removeAttribute("metadatas");
–确定吗?请确保你真的想从seesion删除变量...删两次吗?
LoL#4
ArrayList arrActions = new ArrayList();
HashMap order = new HashMap();
public ArrayList getHighLights(int idStructure, int language, int home) { ...
- 非常感谢你对类型的具体声明。 不过你听过接口和集合API的设计理念没?
LoL#5
ArrayList avlanguages=null;
if (request.getAttribute("avlanguages")!=null){
avlanguages=(ArrayList)request.getAttribute("avlanguages");
}
<%if (avlanguages!=null && avlanguages.size()>0){%>
<%for(int x=0;x<avlanguages.size();x++) { %>
-是的,这是太多的Java代码的JSP页面内,而不是使用JSTL <c:forEach...
LoL#6
一些拼写错误,出现在日志和代码中:
END GENERTAION THEMATICS.
updateStatusDocumentDeplubishCron()
LoL#7
CommonDatosPopUp pop = new CommonDatosPopUp();
pop = (CommonDatosPopUp)popUp[i];
- 这是伟大的!!! 感谢实现了之后,就把它扔掉。
LoL#8
ArrayList List = new ArrayList();
List = (ArrayList) baseManagerDao.getPosition(baseItemIdVar);
for (int i = 0; i < List.size(); i++)
- 再次,创建空ArrayList(),然后把它扔掉
- 变特殊命名List接口看起来像List (我不知道为什么这样的语言允许在这里污染自己)
- 看了这样的代码,你还有其他什么样的感想?
WTF#1
if (cm.getBlockLevels() != null) {
request.setAttribute("blocklevel", cm.getBlockLevels());
}
if (cm.getUsers() != null) {
request.setAttribute("users", cm.getUsers());
}
if (pm.getSigGroupPub() != null) {
request.setAttribute("siggroups", pm.getSigGroupPub());
}
- cm,pm(非常糟糕的局部变量名 – 这两个类是DAO)委托调用DAO的这些方法(很可能访问数据库)被调用两次
- 为什么我们要在第一时间nullcheck? 如果结果是null,我们可以简单地把空(删除)压入request attrs。
WTF#2
搜索“siggroup”。 在同一个controller(2000线),我找到了下面的代码:
request.setAttribute(“siggroups”,pm.getSigGroupPub());
request.setAttribute(“sigGroups”,sigGroups);
request.setAttribute(“SigGroups”,hm.getSigGroups(Integer. ..
- 一致性是程序员的生命之盐...
在模版之中:
List sig = (List) request.getAttribute("sigGroups");
if(sig == null) {
sig = (List) request.getAttribute("SigGroups");
}
- 如果你运气够好的话,你可以找到是哪一个对象
WTF#3
没有javadoc,而且部分应用程序中最关键的是comment使非本土语言,包括非ASCII字符。 需要http://translate.google.com/帮助
WTF#4
Classes with 2000 lines of code methods and 14 level nested ifs. These are usually do-everything controllers which serve many unrelated things from the same codebase (page results, binary downloads, csv). These are usually replicated 10 times with minor differences to accomodate slightly different use cases. Nough said
类文件包含2000行代码的方法和14个嵌套层级的if…else。经常是控制器做一切事情,这类控制器服务于许多来自同一个代码库不相关的事情(页面结果,二进制下载,cvs等)。还经常复制10次代码去适应略微不同的用例。
WTF#5
pubHighlights = cm.getHighLights(structId, userLang,
Constants.PREDEFINED_SEARCH);
– 大家正在利用变化的常量,而不是不同的方法或继承的方法:
public ArrayList getHighLights(int idStructure, String lang, int home) {
...
if (home == 1) {
listado = commonDao.getHightLightHome(idStructure);
} else {
listado = commonDao.getHightLightPredefinedSearch(idStructure);
}
- facepalm for home==1 instead of Constant. good luck with debugging when that constant changes (home == 1, 为什么不使用常量?)
- by the way it turns out that the initial call should send Constants.HOME instead of Constants.PREDEFINED_SEARCH. It just happens that both equals 1.
顺便说下,应该用Constants.HOME来代替Constants.PREDEFINED_SEARCH常量,这两个常量只是凑巧都等于1.
WTF#6
Absense of templating reuse. The 150+ JSP templates contain everything from html declarations to website footer (with copyrights and everything). Only with minor and insignificant differences due to inconsistent copy pasting of headers and whole pages with minor changes. Ever heard of include?
模板缺少复用。多于150个的jsp模板包含从html声明到站点页脚的一切东西。
WTF#7
强大的表单验证, 看了下面的代码,你的感想是什么?:
if (appForm.getFileCV() == null || StringUtils.isEmpty(appForm.getFileCV().getFileName())){
errors.add("fileCV", new ActionError("error.fileCV.required"));
}else if (!appForm.getFileCV().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileCV().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileCV().getFileName().toLowerCase().endsWith(".rtf") && !appForm.getFileCV().getFileName().toLowerCase().endsWith(".sdc")
&& !appForm.getFileCV().getFileName().toLowerCase().endsWith(".zip") )
errorExtension = true;
if (appForm.getFileLetter() == null || StringUtils.isEmpty(appForm.getFileLetter().getFileName())){
errors.add("fileLetter", new ActionError("error.fileLetter.required"));
}else if (!appForm.getFileLetter().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileLetter().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileLetter().getFileName().toLowerCase().endsWith(".rtf") && !appForm.getFileLetter().getFileName().toLowerCase().endsWith(".zip")
&& !appForm.getFileLetter().getFileName().toLowerCase().endsWith(".zip"))
errorExtension = true;
/* if (fileForm == null || StringUtils.isEmpty(fileForm.getFileName())){
errors.add("fileForm", new ActionError("error.fileForm.required"));
}else*/
if(appForm.getFileForm() != null && !StringUtils.isEmpty(appForm.getFileForm().getFileName()))
if (!appForm.getFileForm().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileForm().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileForm().getFileName().toLowerCase().endsWith(".rtf") && !appForm.getFileForm().getFileName().toLowerCase().endsWith(".sdc")
&& !appForm.getFileForm().getFileName().toLowerCase().endsWith(".zip"))
errorExtension = true;
if(appForm.getFileOther1() != null && !StringUtils.isEmpty(appForm.getFileOther1().getFileName()))
if (!appForm.getFileOther1().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileOther1().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileOther1().getFileName().toLowerCase().endsWith(".rtf")&& !appForm.getFileOther1().getFileName().toLowerCase().endsWith(".sdc")
&& !appForm.getFileOther1().getFileName().toLowerCase().endsWith(".zip"))
errorExtension = true;
if(appForm.getFileOther2() != null && !StringUtils.isEmpty(appForm.getFileOther2().getFileName()))
if (!appForm.getFileOther2().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileOther2().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileOther2().getFileName().toLowerCase().endsWith(".rtf")&& !appForm.getFileOther2().getFileName().toLowerCase().endsWith(".sdc")
&& !appForm.getFileOther2().getFileName().toLowerCase().endsWith(".zip"))
errorExtension = true;
if(appForm.getFileOther3() != null && !StringUtils.isEmpty(appForm.getFileOther3().getFileName()))
if (!appForm.getFileOther3().getFileName().toLowerCase().endsWith(".doc") && !appForm.getFileOther3().getFileName().toLowerCase().endsWith(".pdf")
&& !appForm.getFileOther3().getFileName().toLowerCase().endsWith(".rtf")&& !appForm.getFileOther3().getFileName().toLowerCase().endsWith(".sdc")
&& !appForm.getFileOther3().getFileName().toLowerCase().endsWith(".zip"))
errorExtension = true;
WTF#8
There are methods which return Vector (yes, 1999 called) and it turns out that the result is never being used but instead the purpose of the method is to mutate the parameters. Smart.
许多方法返回Vector并且结果证明它们已经不在被使用了。而这些方法存在的目的只是为了达到参数变异效果。聪明(讽刺^_^)。
ShowStopper#1
cm.getHighLights is an expensive operation (calculates the nested menu of the website) and the following code is supposed to introduce caching into the game:
cm.getHighLights是一个代价昂贵的操作(生成网站树状菜单),下面的代码应该是引入缓存:
ArrayList pubHighlights = (ArrayList)request.getSession().getAttribute("publicationsHighlights");
if(pubHighlights == null || pubHighlights.size() == 0)
pubHighlights = cm.getHighLights(structId, userLang, Constants.PREDEFINED_SEARCH);
request.getSession().setAttribute("publicationsHighlights", pubHighlights);
- it stores the result in the http session so there is a lot of waste of memory in case we have many users
把结果保存在http session中,在用户很多的场景下比较浪费内存
- the generated menu takes into account “structId” and “userLang”. The cache key is only one though (“publicationsHighlights” in the session), so if the user changes structId or userLang, the menu stays the same
生成的菜单需要考虑“structId” 和 “userLang”。而缓存的key只用到“publicationsHighlights”一个,因此当用户改变structId 或 userLang,菜单保持不变。
- changes on the menu structure are not reflected to already cached clients. They’ll see these changes only if they get a new session (come back later, use another browser etc)
当菜单结构变化时不会反映到已经缓存的客户端。只有他们获取新的会话才能看到改变的结果。
showstopper#2
Application “does things” to the database on view. Things == if stuff are not there it silently creates them, so for example if you visit the about page of the French site and there is no content there (either from CM error or data corruption) it simply creates an empty one and inserts it into the database. This is nice and useful especially when the application thinks that there isn’t any content there, so after a couple of days you’ll find thousand of empty “about” pages under the French site waiting to be fixed by you.应用程序在视图层做一些数据库操作。
showstopper#3
Total incompetence in exception design and handling. The usual anti-pattern of swallow and return new ArrayList() is followed through the system. Database errors are masked and the system goes on doing what it was doing (e.g continuing with other parts of data changes, email dispatching etc).
showstopper#4
“Change navigation element==edit property” anti-pattern. This is sick. Imagine a CRUD page with a couple of filters on top and an entities listing below. In the year filter you choose 2010 and hit GO. The listing is updated with entries from 2010. Now you change the year filter to 2011 but do not hit GO. Instead you hit EDIT on one of the 2010 entities below. What happens is that the 2011 value from the filter is transfered into the (hidden) element of the edit form. As soon as you hit SUBMIT the entity now belongs on 2011. Nice.
showstopper#5
The search is FUBAR. A single search for “foo” issues 200.000 db queries and requires 5 minutes on the production server because:
- it first does “select *” the whole publications database in sorted batches of 1000 back to a collection.
- it then feeds this collection into a method which filters things out.
- while filtering some entity methods are accessed and due to bad fetch plan from hibernate tons of N+1 statements are executed.
showstopper#6
“Toxic servlets hierarchy”. All actions extend something (a base class) which extends servlet. The base class provides a public static spring factory field which is initialized on boot of the servlet. Yes, the only reason of existence of this base class is to provide this field and the actions extend it in order to get access to this public static field. Great.
“有毒的servlet层次”。所有action继承一个基类,这个基类继承servlet。基类提供一个公开的静态的spring工厂变量,此变量在servlet启动的时候初始化。基类存在的唯一理由就是提供这个变量,而action继承基类就是为了访问变量。太棒了(讽刺^_^)。
showstopper#7
Log4j & hibernate initialization rediscovered! Both libraries are being configured in the following fashion:
- read the log4j.properties and hibernate.cfg.xml configuration files from a custom location using a ContextListener
- write contents into a new file in the server’s root folder
- load from there
- documentation states that if application cannot boot the application’s configuration files should be removed from the server’s root folder!
The end.
分享到:
相关推荐
【标题】"情缘一生全站代码"所指的是一个完整的网站源码集合,可能是由ASP(Active Server Pages)开发的,适用于构建一个以情感、缘分为主题的网站。ASP是一种微软公司的服务器端脚本语言,用于创建动态交互式网页...
歌曲好人一生平安51单片机c语言源代码 可以在51的开发板上利用蜂鸣器播放出来,要求蜂鸣器的效果要好,分辨率要高一点的。
在IT行业中,程序员们以其独特的创意和编程技巧来表达情感,"程序员代码表白-爱你一辈子.zip"就是一个典型的例子。这个压缩包文件包含了程序员们用代码来传递爱意的方式,让我们一起深入探究其中蕴含的IT知识。 ...
专注在自己的领域,活出不一样的大雁一生。 01,打瞌睡02,不改需求03,超人04,打CALL05,发怒06,鬼脸07,开机车08,看书学习09,码代码10,溺水之后11,致敬12,遇到bug Copyright ©️ 暗轩
祝你平安的C51单片机音乐代码,不用打,直接下载便可以啦
《代码大全》是一本深受程序员喜爱的经典编程书籍,由Steve McConnell撰写,全面涵盖了软件开发过程中的编码实践和技术。这本书的PDF版提供了方便的电子阅读体验,使得开发者可以在任何时间、任何地点查阅其中的丰富...
6. **模板**:模板是C++中的泛型编程工具,可以用来创建泛型函数和泛型类,使得代码更加通用,提高代码复用性。 7. **异常处理**:在C++中,异常处理是一种处理运行时错误的方式。通过try、catch和throw语句,你...
・2009年孩子最喜欢的书>> ・弗洛伊德作品精选集59折 ・2010考研英语大纲到货75折... ・权威定本四大名著(人民文... ・口述历史权威唐德刚先生国... ・袁伟民与体坛风云:实话实... ・我们台湾这些年...
- 预测一生的大运和流年运势,包括事业、财富、健康、婚姻等方面。 - 提供命理解读建议,指导用户如何调整生活和心态以趋吉避凶。 在"算命代码V1.1"中,"BK160614"可能是这个软件的某个版本号或者是内部使用的文件...
在这个“C基础代码培训班”中,你将深入理解其核心概念。 首先,我们要了解C语言的基本结构,它由预处理指令、函数定义、变量声明和语句组成。预处理指令包括宏定义、条件编译等,它们在编译前处理源代码。函数是C...
颜色十六进制代码对应表。
游戏里,玩家只需在开局时选天赋、分配初始属性,后面就是看岁月如白驹过隙,转眼就过完了这一生,不满意的话可以点击 再次重开 即刻开启新的人生。该游戏凭借诙谐幽默的文案,和出乎意料的结局,风靡一时。现在除了...
标题中的“一生最辉煌的年龄测试”显然是一款应用软件,其功能是根据用户输入的名字预测他们最辉煌的年龄。这款软件是用C#编程语言编写的,并采用了Windows Forms(Winform)作为用户界面框架。Winform是.NET ...
例如,如果发现某个方法运行时间过长,可以通过分析并提取热点代码,使用更高效的数据结构或算法来提高性能。同时,合理使用并发和多线程技术也能提升运行速度,但需要注意线程安全问题。 代码的可读性和可维护性也...
每个人的虹膜都有独一无二的纹理、斑点和细丝结构,这些特征在人的一生中基本保持不变。因此,虹膜识别被广泛应用于安全系统、门禁控制、移动设备解锁等领域。 MATLAB是一款强大的数学计算和数据分析环境,同时提供...
《代码之美》是一本在IT领域备受推崇的经典著作,它深入浅出地探讨了软件开发中的编程艺术和设计原则。这本书的中文版为中国的程序员和软件工程师提供了更易理解的阅读选择,使得广大的中文读者能够更好地领略到编程...
在快速变化的科技环境中,每个机会都可能是“一生一次”,抓住机遇,把握创新,就可能在职业生涯中实现质的飞跃。因此,持续学习,保持敏锐的洞察力,以及敢于尝试新事物的态度,对于IT专业人员至关重要。 此外,...
本资源“C++代码设计与重用”聚焦于如何编写高效且可复用的C++代码,这对于任何软件开发人员来说都是至关重要的技能。 代码设计是编程过程中的关键步骤,它涉及到程序的结构、模块化以及数据结构的选择。在C++中,...