最近在审核代码,发现有很多问题,把这些问题一一列举出来,算是自己的一个警示。
算是一个系列把。
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.