什么是错题本?
最近一直有热心读者提出:能否找到一个写的不太好的工程例子,然后解析其中的代码错误,让我们能够从中更为直观的体会到糟糕的代码书写。这么多年接触了许多糟糕的代码,但是苦于不能公开,也难以做到。不过,后来接触到了OpenWnn,这是一个很好的工程,它的代码中几乎充斥着各种各样的代码书写坏习惯。就好像中学生的错题本一样,打开一看都是各式各样的错误。足以警醒以后不再犯同样的错误。
本章我们将对OpenWnn的代码问题进行解析,从命名、注释、结构和风格四个方面分别进行阐述。
什么是OpenWnn呢?
我在前面的博客里至少两次提到这个工程。这是一个Android下的开源日文输入法,由Omron Software Co.,Ltd 开发。从Android CupCake开始公开。据说它支持中日英韩四语输入(我只试过日语和英语,没有找到中文和韩语)。
如何才能够获得OpenWnn的源代码呢?
你可以按照Android的源代码下载指南进行下载,也可以在搜索引擎上搜搜找到。
下面是一个可用地址:
http://gitorious.org/sourcecode/google_android_2_2/trees/e850cee/jp/co/omronsoft/openwnn
为什么选择OpenWnn
第一个原因是这段代码整体感觉就是一些没有什么经验的程序爱好者坐在一起写的。
第二个原因是因为他是我所接触过的劣质代码中唯一可以公开的源代码。
在了解为什么它很糟糕之前,请先下载它的代码,并且在Android模拟器或者设备上运行一下看看。
然后对Android输入法的基本概念有所了解。然后再继续阅读下面的内容。否则有些地方可能会没有头绪。
本文的目的在于讨论编码方面的问题,不讨论易用性、软件质量方面的问题。
---------------------分割一下---------------------
一、命名问题
1.否定式命名
严重程度:高
否定式命名让读者阅读的时候必须停顿一下才能够理解。比如if(!mNoInput)就不如if(hasInput())容易理解。
OpenWnn中有好几处否定式命名
DefaultSoftKeyboard.mNoInput 表示没有输入
DefaultSoftKeyboard.mDisableKeyInput 表示禁止键盘输入
DefaultSoftKeyboard.mHardKeyboardHidden 表示硬键盘隐藏
OpenWnnJAJP.onUpdateSelection中的isNotComposing 表示没有选中ComposingText
但是,这并不是说,否定式命名一定不好,比如INVALID_KEYMODE就很好。
2.序号式命名
严重程度:高
序号式命名是诸如var1,var2...varn这样形式的命名,读者无法从1~n中获取什么有价值的信息。
OpenWnn中的序号式命名
ComposingText.LAYER0,LAYER1,LAYER2表示英文->假名->汉字三层转换。
其实命名为LAYER_ALPHABET, LAYER_KANA, LAYER_JAPANESE会更好一些。
3.用词不当
严重程度:低
用词不当是指一个命名用了一个接近于真实意思的词汇,但是并不贴切,有更好的词汇可以表示更贴切的意思。
DefaultSoftKeyboardJAJP.mLimitedKeyMode表示允许切换的受限制的键盘种类数组。实际上Constraint比Limited会更加贴切的表示这个意思。
OpenWnn.LIMIT_INPUT_NUMBER表示最大允许输入长度,实际上,可以命名为MAX_INPUT_LENGTH;因为LIMIT和NUMBER都有其他意思。
4. 词不达意
严重程度:中
用的词汇并不能准确的表达真实意思的,归类为词不达意。词不达意的情况会引起读者的误解。
比如:OpenWnnJAJP.updatePrediction方法中的第一个变量candidates,光看命名它应该是数组,List之类的变量,然而它却是一个int型变量,用来存储候补的个数的。正确的命名是candidatesCount。
5. 言行不一
严重程度:高
言行不一是指方法或者类执行了其声明的作用以外的工作。这种问题很严重,如果别人(包括几个月之后的自己)来接手该代码的时候,打算通过名称复用代码的时候就要十分小心,然而这种十分小心却是人为错误引起的。如果命名和实际作用一致那么准备复用代码的人也就不必过多的考虑了。
DeafultSoftKeyboardJAJP.toggleShiftLock(),同时调用了ChangeKeyboard()方法。
而ChangeKeyboard会更新很多底层数据...这也涉及到结构的问题。将会在结构性分析中详细解析。
6.前缀、后缀使用错误
严重程度:低
前缀、后缀使用错误的时候对阅读会形成一定的障碍(读起来有点怪而已),但是并不影响整体阅读。
has~/is~/等就是用于方法的前缀是不应该用于变量的。
OpenWnnJAJP.mHasContinuedPrediction 想表达是否有后续的预测, 但是这个变量如果命名为mPredictedFurther就可以达到1)缩短命名,2)规避了不当用词的问题。同样的,在TextCandidateViewManager中也有几个类似的命名:mIsFullView,mIsScaleUp。
~Current前缀一般如果出现是和Prev, Next并列出现的,单独的Current没有任何意义。
DefaultSoftKeyboard.mCurrentKeyboard的Current没有意义。
~List, ~Map不应该出现在后缀中,这也许是受了匈牙利命名法的影响,但是命名中附带List,Map是没有必要的。
比如:keyboardList 可以写成keyboards.
7.长命名
严重程度:中
长命名导致阅读费力。这是公认的问题。
OpenWnnJAJP.processKeyEventNoInputCandidateShown(KeyEvent ev)方法当属其中一个
这个命名长的原因是:其中包含了一定的限制条件:No Input + Candidate Shown(参考调用的代码才知道这是两个限制条件)。所以,这个命名其实是结构性问题。这将在结构性问题中详细讨论。这里简单的提一下如何解决这个问题。
其实这段代码是想解决在Candidate显示时,左右键以外的其他非输入性按键的动作的问题。那么,这个应该交给OpenWnn.onKeyDown来处理,这个方法应该将各种按键进行分类,然后交给不同的分类去处理。那么这段处理可能属于FunctionalKey.onKeyDown和ArrowKey.onKeyDown。
8.本地语命名
严重程度:低
本地语即采用只有本地人才能明白的词汇来命名的情况。OpenWnn其中有很多日语命名的情况。除了必要的Hiragana,Katakan之类的命名之外,有些并不应该用本地语来命名。比如isRenbun(这个拼写还有错误,其实想写的是isRenban)表示连续的数字(ContinuousNumbers)
DefaultSoftKeyboard.KEYCODE_QWERTY_ZEN_HIRA
DefaultSoftKeyboard.KEYCODE_QWERTY_HAN_NUM
其中的HAN和ZEN是表示半角和全角的,如果改成HALF和FULL就易懂的多。
其他的还有:EISU_KANA, convHansuuji, convHanEiji, convZenEiji, moji, inputRomaji等。
由于这个是开发的输入法,所以这个问题的严重程度定为:低,其实这个对于其他的读者来说是一个比较大的阅读问题。
9.类似的命名
严重程度:中
命名类似,但是表示不同的意思的情况会引起阅读问题,也会引起调用问题。
OpenWnn.mHardShift, OpenWnn.mShiftPressing, OpenWnnJAJP.mShiftOn这三个到底有什么区别呢?
10.大写字母命名
严重程度:中
采用大写字母进行命名会导致很多问题,包括将来如果采用反射时对于首字母大写的设定时会产生规则例外。OpenWnn中的包JAJP命名和EN命名就属于这种情况。还有WnnPOS类也是这种情况。
11.无意义的命名
严重程度:高
面对无意义的命名,读者无法从中获取有效的信息,也不知道应该如何使用这样的对象(方法、变量)。
KanaConverter中的mStringBuff就是这样一个变量。