2017-11-26 5 views
1

以下のコードでは、JButtonを渡しています。そのラベルはcreateKeyboard()メソッドに渡しています。目標は、正しい順序でJPanelにJButtonを追加して、QWERTYキーボード形式で正しく表示されるようにすることです。ボタンとそれに対応するラベルは、A〜Zから始まるこのメソッドに渡されます。複数の文字列に文字が含まれているかどうかを効率的に確認する

このメソッドは文字をソートし、正しいパネルに追加します。私はこれを行うより効率的な方法があるかどうかを知りたいのですが、今のところ私はこれを達成するために複数のforループを使用していますが、これを行うにはより良い方法が必要であると確信しています。

public void addKeyboard(char c, JButton button) { 

    String QP = "QWERTYUIOP"; 
    String AL = "ASDFGHJKL"; 
    String ZM = "ZXCVBNM"; 

    keyboardQP = new ArrayList<JButton>(); //JButton ArrayLists 
    keyboardAL = new ArrayList<JButton>(); 
    keyboardZM = new ArrayList<JButton>(); 




    for (int i = 0; i < QP.length(); i ++) { 
     if (c == QP.charAt(i)) { 
      keyboardQP.add(button); 
     } 
    } 


    for (int i = 0; i < AL.length(); i ++) { 
     if (c == AL.charAt(i)) { 
      keyboardAL.add(button); 
     } 
    } 

    for (int i = 0; i < ZM.length(); i ++) { 
     if (c == ZM.charAt(i)) { 
      keyboardZM.add(button); 
     } 
    } 
+0

小さな改良点:メソッドが呼び出されるたびに 'String QP =" QWERTYUIOP ";のような文字列を再定義する必要はありません。それぞれを一定にします(静的最終フィールド)。また、各メソッド呼び出しでArrayListを初期化しないでください。 – c0der

答えて

2

。しかし、インスタンス化する際には事前にリストサイズを設定する必要があります。しかし、メソッドのボディの中に新しいArrayListを毎回作成するのは間違っているので、コンストラクタで行う必要があります。そうしないと、複数のボタンを追加できなくなります。

String QP = "QWERTYUIOP"; 
keyboardQP = new ArrayList<>(QP.length()); 

int index; 
if ((index = QP.indexOf(c)) >= 0) 
    keyboardQP.add(index, button); 

この場合、ループする必要はありません。ボタンはその順序に従って追加されます。

0

私はむしろあなたが何ができるか、主

final List<String> QP = Arrays.asList("Q", "W", "E", "R", "T", "Y", "U", "I" , "O", "P"); 
final List<String> AL = Arrays.asList("A", "S", "D", "F", "G", "H", "J", "K", "L"); 

if(QP.contains(c)) { 
    keyboardQP.add(button); 
    break; 
} 

if(AL.contains(c)) { 
    keyboardAL.add(button); 
    break; 
} 
1

読みやすくするために、次のコードを持っていると思いインデックスが文字(下記の例)のint型の値であるブール配列を作成されていますが、 早すぎる最適化はすべての悪の根源です、forループは非常に高速で、必要なときにのみコードを最適化します。コードの可読性は、コードをよりよく維持し、それを何年も後に理解することをより重要にします。

例:あなたはindexOf方法でcharのインデックスを検索し、ArrayList応じてインデックスに追加する

static boolean[] qpArray = new boolean[127]; // do it outsite the 
// method, the jvm will not build the array every method call, when the 
// class will be initialized the static part will be executed. 
static { 
    Arrays.fill(qpArray, Boolean.FALSE); 
    qpArray['Q'] = true; 
    qpArray['W'] = true; 
    qpArray['E'] = true; 
    qpArray['R'] = true; 
    qpArray['T'] = true; 
    qpArray['Y'] = true; 
} 
// do the same for AL ZM... 

public void addKeyboard(char c, JButton button) { 
    final keyboardQP = new ArrayList<JButton>(); 

    if(qpArray[c]) { 
    keyboardQP.add(c); 
    } 

    // do the same for AL ZM... 
} 
0

独自のメソッドを作成し、指定されたパラメータで呼び出します。 3回の代わりにこのメソッドを呼び出す:)

public void adContent(String myString, ArrayList<JButton> myArrayList){ 

    for (int i = 0; i < myString.length(); i ++) { 
     if (c == myString.charAt(i)) { 
      myArrayList.add(button); 
     } 
    } 
} 
0

A非常に可読ではなく、マイクロ最適化されたものの変形例:

private static final String QP = "QWERTYUIOP"; 
private static final String AL = "ASDFGHJKL"; 
private static final String ZM = "ZXCVBNM"; 

private List<JButton> keyboardQP = new ArrayList<JButton>(); 
private List<JButton> keyboardAL = new ArrayList<JButton>(); 
private List<JButton> keyboardZM = new ArrayList<JButton>(); 

public void addKeyboard(char c, JButton button) { 
    String s = String.valueOf(c); 
    if (QP.contains(s)) { 
     keyboardQP.add(button); 
    } else if (AL.contains(s)) { 
     keyboardAL.add(button); 
    } else if (ZM.contains(s)) { 
     keyboardZM.add(button); 
    } 

} 
+0

メソッドが呼び出されるたびに 'String QP =" QWERTYUIOP ";のような文字列を再定義する必要はありません。それぞれを一定にします(静的最終フィールド)。また、各メソッド呼び出しでArrayListを初期化しないでください。 – c0der

+0

十分に、私はここでforループを取り除くことに焦点を当てており、残りのコードはそのまま残しました。 – NorthernSky

+0

編集を検討する – c0der

0

マイクロ最適化も読み取れない変異体ものの:

private static int[] mapping = new int[91]; 
static { 
    mapping['Q'] = 1; 
    mapping['W'] = 1; 
    mapping['E'] = 1; 
    mapping['R'] = 1; 
    mapping['T'] = 1; 
    mapping['Y'] = 1; 
    mapping['U'] = 1; 
    mapping['I'] = 1; 
    mapping['O'] = 1; 
    mapping['P'] = 1; 
    mapping['A'] = 2; 
    mapping['S'] = 2; 
    mapping['D'] = 2; 
    mapping['F'] = 2; 
    mapping['G'] = 2; 
    mapping['H'] = 2; 
    mapping['J'] = 2; 
    mapping['K'] = 2; 
    mapping['L'] = 2; 
    mapping['Z'] = 3; 
    mapping['X'] = 3; 
    mapping['C'] = 3; 
    mapping['V'] = 3; 
    mapping['B'] = 3; 
    mapping['N'] = 3; 
    mapping['M'] = 3; 
} 

private List<JButton> keyboardQP = new ArrayList<JButton>(); 
private List<JButton> keyboardAL = new ArrayList<JButton>(); 
private List<JButton> keyboardZM = new ArrayList<JButton>(); 

public void addKeyboard(char c, JButton button) { 
    switch (mapping[c]) { 
     case 1: 
      keyboardQP.add(button); 
      break; 
     case 2: 
      keyboardAL.add(button); 
      break; 
     case 3: 
      keyboardZM.add(button); 
      break; 
    }  

} 
0

配列リストを作成するための単純なJava 8の代替方法:

String AL = "ASDFGHJKL"; 
    List<JButton> keyboardAL = AL.chars() 
           .mapToObj(i -> (char)i) 
           .map(ch -> String.valueOf(ch)) 
           .map(JButton::new) 
           .collect(Collectors.toList()); 

これは、クラスレベルで1回実行する必要があり、各メソッド呼び出しでは実行しないでください。