0

異なる操作を実行するための 2 つの検索ループがありますが、これがいかに反復的であるかに不満があります。

アイテムを削除するために使用される最初の方法は次のとおりです。

public void RemovePlayer(int theID){
    boolean matchFound = false;

    if (playerObjects.size() != 0){
        for (int i = 0; i < playerObjects.size(); i++){
            Person playerToRemove = (Person) playerObjects.get(i);
            if (playerToRemove.getID() == theID){
                playerObjects.remove(i);
                System.out.println("Player with ID # " + theID + " removed");
                matchFound = true;
                // As ID is unique, once a player is found it is unnecessary to continue looping
                break;
            }

            // If matchFound is never set to true then show appropriate error
            if (matchFound == false) {
                System.out.println("Player with ID # " + theID + " not found");
            }
        }
    }
    else {
        System.out.println("No players have been added.");    
    }
}

2 番目のメソッドは、基本的に同じコードですが、一致が見つかった場合に別のアクションを実行します。

public void RetrievePlayer(int theID){
    boolean matchFound = false;

    if (playerObjects.size() != 0){
        for (int i = 0; i < playerObjects.size(); i++){
            Person playerToRetrieve = (Person) playerObjects.get(i);
            if (playerToRetrieve.getID() == theID){
                System.out.println("PLAYER FOUND:");
                playerToRetrieve.GetDetails();
                matchFound = true;
                break;
            }

            // If matchFound is never set to true then show appropriate error
            if (matchFound == false) {
                System.out.println("Player with ID # " + theID + " not found");
            }
        }
    } else {
        System.out.println("No players have been added.");    
    }
}

これをリファクタリングするにはどうすればよいですか?

4

3 に答える 3

3

プレイヤーのインデックスを返すメソッド「FindPlayer」はiいかがですか?その場合、RemovePlayer と RetrievePlayer は次のようになります。

public void RemovePlayer(int theID){
    int playerId = FindPlayer(theID);
    if (playerId >= 0) {
        playerObjects.remove(playerId);
    }
}

public void RetrievePlayer(int theID){
    int playerId = FindPlayer(theID);
    if (playerId >= 0) {
        Person player = (Person) playerObjects.get(playerId);
        player.getDetails();
    }
}

その「FindPlayer」メソッドは、次のようになります。

protected int FindPlayer(int theID){
    if (playerObjects.size() != 0){
        for (int i = 0; i < playerObjects.size(); i++){
            Person player = (Person) playerObjects.get(i);
            if (player.getID() == theID){
                return i;
            }
        }

        System.out.println("Player with ID # " + theID + " not found");
    } else {
        System.out.println("No players have been added.");    
    }

    return -1;
}
于 2012-05-01T07:33:27.953 に答える
1

プレーヤーを に入れMap<Integer,Player>ます。次に、リストをループする代わりに、 Map の基本メソッド ( put、 ) を使用します。remove

于 2012-05-01T08:14:25.377 に答える
0

分割すると、をfind返すメソッドと、プレーヤーを引数として取るメソッドを持つことができます。インデックスは一時的なものである可能性があるため(たとえば、他の誰かがリストに追加した場合、インデックスが無効になる可能性があるため)、インデックスを返すよりもこれを好むでしょう。Playerremove

できることは他にもありますが (おそらく、述語を使用しfindAllたメソッドまたはfilterメソッド)、そうする理由があるまでこれらを調べません (You Ain't going to need it)。

于 2012-05-01T07:34:10.443 に答える