Incorrect programming question book -- resolve openwnn (3) Structure Problems

The structure problem is the most serious problem in all problems. Many problems here reflect the author'sLack of basic knowledge.

CodeThe structure is like the structure of a house. It is a framework supporting the entire code. If the structure of the house is not strong enough, the House will collapse. If the code structure is not good enough, it will be expensive to modify bugs or change the corresponding requirements.

1. the MVC structure is missing

Severity: very high

Input Methods and other applicationsProgramIn the same way, it is composed of: Data (model), view (View) and controller (control). However, openwnn does not classify the packages according to MVC, and it is difficult to distinguish which part of the code is data, that part of the code is a view.

2. Incorrect subordination

Severity: very high

There should be some subordination between various objects, such as: books have bookshelves and books on bookshelves. It cannot be said that there is a library in the book, although it may be a book about the library.

The openwnn class contains the inputviewmanager object definition, while the inputviewmanager implementation class defaultsoftkeyboard contains the openwnn mwnn object definition. This shows that the code designer did not figure out the subordination between the two.

Similarly, for the shift/ALT status, converter type, engine type, vibrator, and sound configurations, data is mixed and stored together, which is also a manifestation of unclear subordination.

3. The package structure is randomly divided

Severity: high

The random Package Structure Division includes incorrect package structure division and incorrect class ownership packages.

Openwnn does not have a package structure. Although there are two sub-packages, jajp and EN, the dictionary, setting, and translation should belong to different packages. It is difficult to describe that they belong to the same category.

 

4. High Coupling

Severity: high

The degree of coupling is directly related to the number of bugs, so high cohesion and low coupling are the goal of programmers.

Openwnnevent in openwnn is a message mechanism. Originally, the message mechanism is used to decouple and separate two irrelevant objects that need to be operated on each other. The message mechanism is used as the intermediary to transmit and execute information. However, in openwnn, this event is only an event. Instead of reducing coupling, it increases coupling. The defaultsoftkeyboard. mwnn object is defined for this purpose.

5. An error occurred while inheriting the link.

Severity: very high

Symbollist is inherited fromWnnengineIn fact, this class has nothing to do with translation.

An inheritance Link error may cause expansion difficulties. In addition, symbollist does not reflect any engine information.

 

6. array Redundancy

Severity: high

If an array is defined to be larger than the actual size, but no value is assigned to the excess part, nullpointerexception occurs when the data is accessed.

Defasoftsoftkeyboard.MkeyboardThis is an array of this type. It uses a 6-dimensional array to store data. The penultimate two-dimensional is max of key modes, which is defined as 7. But in fact, different modes are not all 7 types of keyboards, which forms an empty bucket.

 

7. Lengthy Branch Structure

Severity: high

Many branch structures indicate many conditions, many processing paths, high Code complexity, and more bugs. In addition, the lengthy branch structure leads to a long method length. Scroll the scroll bar constantly during reading. When debugging a program, if you want to find the processing at the bottom, you also need to search.

Defaultsoftkeyboardjajp. onkey adopts a very long branch structure.

8. application error in singleton Mode

Severity: high

The Singleton mode is used to ensure that only one unique object exists during global access, so as to ensure the access accuracy.

Openwnn. mself is implemented in the single-profit mode, but it adopts the private definition, indicating that only it calls it by itself. This is completely unnecessary. Use this.

9. Extra parameter transfer

Severity: Medium

Passing redundant parameters does not cause bugs, but passing redundant parameters does cause problems for calling.

Openwnnjajp. Constructor (context ). The context parameter is unnecessary.

10. the return value adopts global variables.

Severity: Medium

The Return Value of the method should not use global variables. If the method affects the value of the global variable, you can directly access the global variable externally. You do not need to use the return value for the method.

Openwnnjajp. committext ();

The following is a code snippet, where mstatus is a global variable.

 1   Private  Int Committext ( Boolean  Learn ){  2            If  (Isenglishprediction ()){  3   Mcomposingtext. setcursor (composingtext. layer1,  4   Mcomposingtext. Size (composingtext. layer1 ));  5   }  6            Int Layer =Mtargetlayer;  7            Int Cursor = Mcomposingtext. getcursor (layer );  8            If (Cursor = 0 ){  9                Return  Mstatus;  10 }

This statement indicates that there is a problem with the processing in the Code, while mstatus indicates the status, and the return value of committext also indicates the status. Therefore, this method can be changed to the void type, and then the mstatus can be directly used in the call.

11. Unnecessary Guard Code

Severity: Medium

Guard Code is code that prevents exceptions, such
If (instance! = NULL) to prevent nullpointerexception;
If (array. length> = accessindex) prevents arrayindexoutofboundsexception;
If (STR! = NULL & Str. Length ()> end) prevents stringindexoutofboundsexception;

And so on ..

However, if the code structure is well designed, the instance can be ensured not to be empty, and the array can be ensured not to cross-border

The mcurrentkeyboard in the following code of defasoftsoftkeyboard can be processed to ensure that it is not empty. For this code, the mcurrentkeyboard may be empty. When the system is started, an interface is unavailable. If you define an invisiblekeyboard class, assign the mcurrentkeyboard to new invisiblekeyboard.

1PublicView getcurrentview (){2If(Mcurrentkeyboard =Null){3Return Null;4 }5ReturnMmainview;6}
 

 

12. Definition of redundant Constants

Severity: Medium

Due to the poor definition of the structure, it leads to a long list of constants. You always want to skip this part during reading.

In openwnn, keys on various keyboards are defined using integer code, which becomes a bunch of constants in the code.

In fact, this can avoid Code definition problems through structural improvement.

For example, the XML file of the QWERTY keyboard is defined as: <row keys = "q | w | E | r | T | Y | u | I | o | P">, therefore, you do not need to care about the code of each button.

It is worth noting that even Google Pinyin input methods do not use the Android-defined Keyboard XML structure. It can be seen that openwnn uses the android keyboard XML structure, which is really not a brilliant idea.

13. Redundant temporary variables

Severity: Medium

Temporary variables are used to assistAlgorithmBut sometimes the introduction of temporary variables increases the complexity of the Code.

For example, defasoftsoftkeyboardjajp. nextkeymode.

In this case, finding the next mode of the current mode is still a structural issue, not a found issue in a specific algorithm.

14. Default

Severity: high

Anyone who likes to use the switch-case structure knows that default processing is required. But what does default do for a code with a fixed value range? Many people simply write default: break; and put it there. However, such default may become a potential bug source. For example, the language type of the keyboard is defined as 7. What if it is passed into 8?

If the class definition is used and the factory is used to generate the corresponding class instance, no cross-border occurs.

15. Determination of unknown meaning

Severity: high

The judgment in the code is used as the condition for processing the branch, butIf(Parent.Mcomposingtext.Size(1)=0){What does = 0 in this code mean? If you extract the decision, change it to If (! Hascomposing () {} is much easier to understand. Hascomposing is defined

1 Private BooleanHascomposing (){2ReturnParent. mcomposingtext. Size (1)> 0;3}
 

16. Complicated Algorithms

Severity: high

Complex algorithms increase the number of lines of code in vain, making reading difficult. For example, the following process.

 1   Private   Int Gettableindex ( Int  Keycode ){  2           Int Index = 3 (Keycode = keycode_jp12_1 )? 0 :  4 (Keycode = keycode_jp12_2 )? 1 :  5 (Keycode = keycode_jp12_3 )? 2 :  6 (Keycode = keycode_jp12_4 )? 3 :  7 (Keycode = keycode_jp12_5 )? 4 :  8 (Keycode = keycode_jp12_6 )? 5 :  9 (Keycode = keycode_jp12_7 )? 6 :  10 (Keycode = keycode_jp12_8 )? 7 :  11 (Keycode = keycode_jp12_9 )? 8 :  12 (Keycode = keycode_jp12_0 )? 9 :  13 (Keycode = keycode_jp12_sharp )? 10 : 14 (Keycode = keycode_jp12_aster )? 11 :  15 0 ;  16            Return  Index;  17   }  18   

This code can be simplified:

1 Private IntGettableindex (IntKeycode ){2ReturnKeycode-Keycode_jp12_1;3}
 

Similarly, openwnn. searchtogglecharacter is a logically complex code.

Original code:

 1       Protected String searchtogglecharacter (string prevchar, string [] toggletable, Boolean  Reverse ){  2            For (Int I = 0; I <toggletable. length; I ++ ){  3                If  (Prevchar. Equals (toggletable [I]) {  4                    If  (Reverse ){  5 I -- ;  6                        If (I <0 ){  7                            Return Toggletable [toggletable. Length-1 ];  8 } Else  {  9                            Return  Toggletable [I];  10   }  11 } Else  {  12 I ++ ; 13                        If (I = Toggletable. Length ){  14                            Return Toggletable [0 ];  15 } Else  {  16                            Return  Toggletable [I];  17   }  18  }  19   }  20   }  21            Return   Null  ;  22 }

 

You can simply write it as follows:

 1   Protected String searchtogglecharacter (string prevchar, string [] toggletable, Boolean  Reverse ){ 2       For ( Int I = 0; I <toggletable. length; I ++ ){  3           If  (Prevchar. Equals (toggletable [I]) {  4               Return Toggletable [(I + (reverse? 1:-1) + toggletable. Length) % Toggletable. Length];  5   }  6  }
7Return null;  8 }

 

17. The same meaning is represented by several variables.

Severity: very high

If multiple variables are used to represent the same meaning, inconsistency may occur. Inconsistency may cause serious bugs.

There are many such cases in openwnn.

A. In addition to keymode, the English mode also has an isenglishprediction-engine mode.
B. Candidate full screen besides textcandidatesviewmanager.MviewtypeBesides textcandidatesviewmanager.Misfullview.
C. Apart from defasoftsoftkeyboard.Besides mdisplaymode, there is textcandidatesviewmanager.Mportrait.

 

18. Useless private methods

Severity: low

The private method can only be used inside the class. However, if the private method only returns an internal variable, the definition of this method is superfluous.

Textcandidatesviewmanager.GetcandidateminimumwidthAndTextcandidatesviewmanager.GetcandidateminimumheightThe method is of this type.

 

19. Use the wrong API

Severity: Medium

Some APIs have certain restrictions, but if you do not know when using them, then Bugs will be left in the system.

Textcandidatesviewmanager in openwnn.Mformat is used to format the numbers with commas (,) in the output. However, when the number size exceeds a certain limit, it is rounded down. I am afraid this is a situation that the original author does not know. In addition, I'm afraid I won't get the correct number when it comes to the decimal point.

20. Output Parameters

Severity: Medium

As a feature of the Java language, parameters can be used as output parameters, that is, internal values of parameters can be changed within the method, and results can be produced.

However, if the output parameters are not explicitly marked, a call error may occur, resulting in incorrect data changes.

Kanaconverter. createcandidatestring is such a method. Its third parameter, stringbuffer, is an output parameter. However, this is not described in its method declaration and annotations. If the signature type of this method is changed to the following, it will be better understood.

 

 
PrivateStringbuffer createcandidatestring (string input,
Hashmap <string, string> map)ThrowsInconvertableexception
 

Inconvertableexception is a custom exception that replaces the original Boolean return value.