如何写好代码
最近在审核代码,发现有很多问题,把这些问题一一列举出来,算是自己的一个警示。
算是一个系列把。
1. 不要直接调用某个class中的变量,需要提供一个方法给客户端调用。
比如在某个class里面有errormap,定义如下:
A:
static {
map.put("0", R.string.code0);
map.put("2001", R.string.code2001);
map.put("2002", R.string.code2002);
map.put("2003", R.string.code2003);
map.put("2004", R.string.code2004);
map.put("2005", R.string.code2005);
map.put("2006", R.string.code2006);
map.put("2007", R.string.code2007);
map.put("2008", R.string.code2008);
map.put("2009", R.string.code2009);
map.put("2010", R.string.code2010);
map.put("2011", R.string.code2011);
map.put("2012", R.string.code2012);
map.put("2013", R.string.code2013);
map.put("2014", R.string.code2014);
map.put("2015", R.string.code2015);
map.put("2016", R.string.code2016);
map.put("2017", R.string.code2017);
map.put("2018", R.string.code2018);
map.put("2019", R.string.code2019);
map.put("2020", R.string.code2020);
map.put("2021", R.string.code2021);
map.put("2022", R.string.code2022);
map.put("2023", R.string.code2023);
map.put("2024", R.string.code2024);
map.put("2025", R.string.code2025);
map.put("2026", R.string.code2026);
map.put("2027", R.string.code2027);
map.put("2028", R.string.code2028);
map.put("2029", R.string.code2029);
map.put("2999", R.string.code2999);
map.put("3001", R.string.code3001);
map.put("3002", R.string.code3002);
map.put("3003", R.string.code3003);
map.put("3004", R.string.code3004);
map.put("3010", R.string.code3010);
map.put("3100", R.string.code3100);
map.put("3101", R.string.code3101);
map.put("3102", R.string.code3102);
map.put("3103", R.string.code3103);
map.put("3104", R.string.code3104);
map.put("3105", R.string.code3105);
map.put("3106", R.string.code3106);
map.put("3107", R.string.code3107);
map.put("3108", R.string.code3108);
map.put("3109", R.string.code3109);
map.put("3110", R.string.code3110);
map.put("3111", R.string.code3111);
map.put("3112", R.string.code3112);
map.put("3113", R.string.code3113);
map.put("3114", R.string.code3114);
map.put("3115", R.string.code3115);
map.put("3116", R.string.code3116);
map.put("3117", R.string.code3117);
map.put("3118", R.string.code3118);
map.put("3119", R.string.code3119);
map.put("3120", R.string.code3120);
map.put("3121", R.string.code3121);
map.put("3122", R.string.code3122);
map.put("3123", R.string.code3123);
map.put("3124", R.string.code3124);
map.put("3125", R.string.code3125);
map.put("3126", R.string.code3126);
map.put("3127", R.string.code3127);
map.put("3128", R.string.code3128);
map.put("3129", R.string.code3129);
map.put("3130", R.string.code3130);
map.put("3131", R.string.code3131);
map.put("3132", R.string.code3132);
map.put("3133", R.string.code3133);
map.put("3134", R.string.code3134);
map.put("3135", R.string.code3135);
map.put("3136", R.string.code3136);
map.put("3137", R.string.code3137);
map.put("3138", R.string.code3138);
map.put("3139", R.string.code3139);
map.put("3140", R.string.code3140);
map.put("3141", R.string.code3141);
map.put("3142", R.string.code3142);
map.put("3143", R.string.code3143);
map.put("3144", R.string.code3144);
map.put("3145", R.string.code3145);
map.put("3146", R.string.code3146);
map.put("3147", R.string.code3147);
map.put("3148", R.string.code3148);
map.put("3149", R.string.code3149);
map.put("3150", R.string.code3150);
map.put("3151", R.string.code3151);
map.put("3152", R.string.code3152);
map.put("3153", R.string.code3153);
map.put("3154", R.string.code3154);
map.put("3155", R.string.code3155);
map.put("3156", R.string.code3156);
map.put("3157", R.string.code3157);
map.put("3158", R.string.code3158);
map.put("3159", R.string.code3159);
map.put("3160", R.string.code3160);
map.put("3161", R.string.code3161);
map.put("3162", R.string.code3162);
map.put("3163", R.string.code3163);
map.put("3164", R.string.code3164);
map.put("3165", R.string.code3165);
map.put("3166", R.string.code3166);
map.put("3167", R.string.code3167);
map.put("3168", R.string.code3168);
map.put("3169", R.string.code3169);
map.put("3170", R.string.code3170);
map.put("3171", R.string.code3171);
map.put("3172", R.string.code3172);
map.put("3173", R.string.code3173);
map.put("3174", R.string.code3174);
map.put("3175", R.string.code3175);
map.put("3176", R.string.code3176);
map.put("3177", R.string.code3177);
map.put("3178", R.string.code3178);
map.put("3179", R.string.code3179);
map.put("3180", R.string.code3180);
map.put("3181", R.string.code3181);
map.put("3182", R.string.code3182);
map.put("3183", R.string.code3183);
map.put("3184", R.string.code3184);
map.put("3185", R.string.code3185);
map.put("3186", R.string.code3186);
map.put("3187", R.string.code3187);
map.put("3188", R.string.code3188);
map.put("3189", R.string.code3189);
map.put("3190", R.string.code3190);
map.put("3191", R.string.code3191);
map.put("3192", R.string.code3192);
map.put("3193", R.string.code3193);
map.put("3194", R.string.code3194);
map.put("3195", R.string.code3195);
map.put("3196", R.string.code3196);
map.put("3197", R.string.code3197);
map.put("3198", R.string.code3198);
map.put("3199", R.string.code3199);
map.put("3200", R.string.code3200);
map.put("3201", R.string.code3201);
map.put("3202", R.string.code3202);
map.put("3203", R.string.code3203);
map.put("3204", R.string.code3204);
map.put("3204", R.string.code3204);
map.put("3205", R.string.code3205);
map.put("3206", R.string.code3206);
map.put("3207", R.string.code3207);
map.put("3208", R.string.code3208);
map.put("3209", R.string.code3209);
map.put("3210", R.string.code3210);
map.put("3211", R.string.code3211);
map.put("3212", R.string.code3212);
map.put("3213", R.string.code3213);
map.put("3214", R.string.code3214);
map.put("3215", R.string.code3215);
map.put("3216", R.string.code3216);
map.put("3217", R.string.code3217);
map.put("3218", R.string.code3218);
map.put("3219", R.string.code3219);
map.put("3220", R.string.code3220);
map.put("3221", R.string.code3221);
map.put("3222", R.string.code3222);
map.put("3223", R.string.code3223);
map.put("3224", R.string.code3224);
map.put("3225", R.string.code3225);
map.put("3226", R.string.code3226);
map.put("3227", R.string.code3227);
map.put("3228", R.string.code3228);
map.put("3229", R.string.code3229);
map.put("3230", R.string.code3230);
map.put("3231", R.string.code3231);
map.put("3232", R.string.code3232);
map.put("3233", R.string.code3233);
map.put("3234", R.string.code3234);
map.put("3235", R.string.code3235);
map.put("3236", R.string.code3236);
map.put("2046", R.string.code2046);
map.put("3999", R.string.code3999);
map.put("999999", R.string.code999999);
map.put("000000", R.string.code000000);
map.put("100000", R.string.code100000);
map.put("10000", R.string.code100000);
map.put("10002", R.string.data_get_fails);
map.put("10001", R.string.dataparseError);
map.put("downloadcode1", R.string.downloadcode1);
}
B: 如果直接通过
showAddFavoriteDialog(Shdebug.map.get(resultCode), context);
来调用res id,这样很容易导致null pointer exception:
1. resultcode =null, resultcode 不是数字,resultcode 没有在map定义,
但是如果同一个方法访问
public static int getAlertDialogMessageId(String resultCode) {
int resint;
if (resultCode == null || "".equalsIgnoreCase(resultCode)) {
resint = R.string.code100000;
} else {
resint = R.string.code10086;
}
try {
if (map.containsKey(resultCode)) {
resint = (Integer.valueOf(map.get(resultCode))).intValue();
}
} catch (Exception e) {
resint = R.string.code10086;
e.printStackTrace();
}
return resint;
}
2. 如果出现重复的代码,应该尽量将其抽象成一个统一的一个方法
如:
btn = new Button(this);
btn.setId(101);
btn.setText("start service");
btn.setBackgroundResource(R.drawable.heart);
btn.setOnClickListener(this);
LinearLayout.LayoutParams param = new LinearLayout.LayoutParams(120, 50);
param.topMargin = 10;
layout.addView(btn, param);
btn2 = new Button(this);
btn2.setId(102);
btn2.setText("bind service");
btn2.setBackgroundResource(R.drawable.heart);
btn2.setOnClickListener(this);
layout.addView(btn2, param);
btn3 = new Button(this);
btn3.setId(103);
btn3.setText("IPC call");
btn3.setBackgroundResource(R.drawable.heart);
btn3.setOnClickListener(this);
layout.addView(btn3, param);
btn4 = new Button(this);
btn4.setId(104);
btn4.setText("exit");
btn4.setBackgroundResource(R.drawable.heart);
btn4.setOnClickListener(this);
layout.addView(btn4, param);
设置每个btn几乎很相似,抽象成如下的方法:
private void setBtn(LinearLayout layout, Button btn, String text, int resId) {
btn = new Button(this);
btn.setId(resId);
btn.setText(text);
btn.setBackgroundResource(R.drawable.music_play);
btn.setOnClickListener(this);
layout.addView(btn, getPara());
}
调用的话:
setBtn(layout, btnStartService, STRSTARTSERVICE, STARTSERVICE);
setBtn(layout, btnBindService, STRBINDSERVICE, BINDSERVICE);
setBtn(layout, btnIPCCall, STRIPCCALL, IPCCALL);
setBtn(layout, btnExit, STREXIT, EXIT);
setBtn(layout, btnTransferTOIPCSync, STRIPCSYNC, IPCSYNC);
3. 不要使用没有意义的magic number。
在代码中0,1,-1,100只有计算机可以看懂,如果要给别人维护,还是用人可以看懂的语言把:
public void onClick(View v) {
switch (v.getId()) {
case 101:
startService();
break;
case 102:
bindService();
break;
case 103:
transact();
break;
case 104:
finish();
break;
}
}
如果变成,是不是更加容易上手。
public void onClick(View v) {
switch (v.getId()) {
case STARTSERVICE:
startService();
break;
case BINDSERVICE:
bindService();
break;
case IPCCALL:
transact();
break;
case EXIT:
finish();
break;
case IPCSYNC:
transfer();
break;
}
}
4.如果在一个activity中有多个button,不要给button注册listener,如下
btn_onlineread.setOnClickListener(ocl_readtxtCatalog);
if(isContinueRead){
btn_onlineread.setText(R.string.continueRead);
}
txt_chapterName1.setOnClickListener(ocl_readtxtCatalog);
这样维护者来说简直是一个恶魔,应该他们要到每个listener中取看代码,一个很找到入口,二是
如果用户狂点狂按,没有办法控制,应该将每个能点击的button或者view一个统一的入口。
private void CommonOnClickEvent(String param, View view) {
if (multiClickFilter.isViewClickIgnored()) {
UtilTools.printstr("multiple click fillter");
return;
}
switch (view.getId()) {
case VIEWFASCICLEID:
sendRequest(BookDetailActionCode.REQ_FASCICLELIST, null);
break;
case VIEWADDFAVORITEID:
sendRequest(BookDetailActionCode.REQ_ADDFAVORITE, null);
break;
case VIEWSUBSRIBEUPDATEID:
sendRequest(BookDetailActionCode.REQ_BOOKUPDATE, null);
break;
case VIEWRECOMMENTBOOKID:
sendRequest(BookDetailActionCode.REQ_GETCONTENTINFO, param);
break;
case VIEWRECENTLYREADID:
sendRequest(BookDetailActionCode.REQ_READRECENTLYREAD, null);
break;
case VIEWONLINECHAPTERID:
sendRequest(BookDetailActionCode.REQ_OPENCHAPTER, param);
break;
case VIEWBOOKDOWNLOADID:
sendRequest(BookDetailActionCode.REQ_DOWNLOADSTATUS, param);
break;
case VIEWRECOMMENDID:
sendRequest(BookDetailActionCode.REQ_RECOMMENDBOOK, null);
break;
case VIEWSIMILIARID:
sendRequest(BookDetailActionCode.REQ_SIMILIARBOOK, null);
break;
case VIEWSHOWTOC:
sendRequest(BookDetailActionCode.REQ_SHOWTOC, null);
break;
case VIEWREGETCONTENTINFOID:
sendRequest(BookDetailActionCode.REQ_REGETCONTENTINFO, param);
break;
}
}
5. 方法中参数的个数不能超过5, 如果超过5个应该用对象来替代
HttpRequestObj obj = new HttpRequestObj(orgcontext, "", handler, action, null, reqBody, null);
ActivityController.forward(obj);
6. 每个方法不能超过50行。如果超过50行,应该将其分解。而且每个方法最好用动词+名词
如:getPhoneNum 就是比较好的命名,每个方法前面都定义内部变量
7. 如果在一个方法中存在连续3个或者3个以上if/else,可以考虑重构,重构方法有很多中,有多态,一般我使用map:如下所示:
public static int RetrieveBookStatus(String contentId, String downloadType) {
int sid = 0;
DownLoadDBAdapter download = new DownLoadDBAdapter(FPAndroidApplication.Instance());
download.open();
String downloadStatus = download.RetrieveBookDownloadStatus(contentId, downloadType);
download.close();
if (downloadStatus != null && !"".equalsIgnoreCase(downloadStatus)) {
if (Shdebug.DOWNLOAD_STATUS_DOWNLOADING.equals(downloadStatus)) {
sid = R.string.downloading_book;
} else if (Shdebug.DOWNLOAD_STATUS_DOWNLOADED.equals(downloadStatus)) {
sid = R.string.bookhasbeendownload;
} else if (Shdebug.DOWNLOAD_STATUS_UNDOWNLOADFULLY.equals(downloadStatus))
sid = R.string.indownloadpool;
else if (Shdebug.DOWNLOAD_STATUS_WAITING.equals(downloadStatus))
sid = R.string.indownloadwaiting;
}
return sid;
}
看到这么多的if/else,是不是有点烦,而且这种关系一般是key-value的关系,我们可以建一个map,如下:
private static Map<Integer, String[]> setDownloadGroup() {
if (downloadGroup == null) {
downloadGroup = new HashMap<Integer, String[]>();
downloadGroup.put(DownloadGroup.ADOWNLOADSUCCESSED, downloadSuccessed);
downloadGroup.put(DownloadGroup.ADOWNLOADRESUMING, downloadResuming);
downloadGroup.put(DownloadGroup.ADOWNLOADUNDERTAKING, downloadUnderTaking);
}
return downloadGroup;
}
要用时:
public static Boolean verifyDownloadType(String downloadicon, int downloadType) {
Boolean downloadFlag = false;
if (downloadicon != null) {
setDownloadGroup();
String[] download = downloadGroup.get(downloadType);
for (int start = 0; start < download.length; start++) {
if (downloadicon.equalsIgnoreCase(download[start])) {
downloadFlag = true;
break;
}
}
}
return downloadFlag;
}
8. 尽量使用primitive type, 少使用reference,站在内存的角度来说, Boolean使用的内存是boolean使用的二倍,
假设一个object 里面有4 Boolean,一共有1000个这样的对象,多使用内存:4*1000/1024 这样就多了4k的空间,
同理,能用int就不用integer,因为使用上差不多,但是如果对象构成一个数组,内存差异会很多,对于嵌入式产品这些
都是很致命的。
9. 多使用解释性变量:
com.fp.app.book.ChapterInfo recentlyReadchapterInfo = bookToc.getChapterInfoByIdx(chapterIndex); Boolean isFree = ChapterStatus.FREE == recentlyReadchapterInfo.status; charpterType = isFree ? "" : Chapter.bookchargeType.undownloaded.toString();
10.
浙公网安备 33010602011771号