自作のSortクラスを作ってみました。よりかっこいいソースを書くためのコツを教えて下さい。

あと、自分の勉強のために追加したほうがいいメソッド等あればコメントお願いします。

import java.util.*;
public class Sort_test
{
    public static void main(String[] args)
    {
        for(String s:args)
        {
            System.out.print(s+" ");
        }

        System.out.println("n並び替えます");
        Comparator Obj = new MyClass();
        sort(args,Obj);

        for(String s:args)
        {
            System.out.print(s+" ");
        }
    }

    public static Object sort(String[] args)
    {
        for(int k=0;k0)
                {
                    String w=args[i];
                    args[i]=args[i+1];
                    args[i+1]=w;
                }
            }
        }
        return args;
    }

    public static Object sort(String[] args,Comparator Obj)
    {
        for(int k=0;k0)
                {
                    String w=args[i];
                    args[i]=args[i+1];
                    args[i+1]=w;
                }
            }
        }
        return args;
    }

    class MyClass implements Comparator
    {
        public int compare(String s1,String s2)
        {
            return s1.compareTo(s2);
        }
    }

    class ReMyClass implements Comparator
    {
        public int compare(String s1,String s2)
        {
            return s2.compareTo(s1);
        }
    }
}

カテゴリー: 技術情報

6件のコメント

mori · 2007-05-31 17:45

とりあえず、インデントをちゃんと合わせましょう。
なんだか、カッコの終わりがすごいことになっているので。

ところで、このコードってちゃんと実行可能ですか?
なんだか、配列へのアクセスで落ちそうな気が、しますけど。

ちなみに、このような方法で行うソートをバブルソートといいますが、このコードは無駄があります。 調べてみませう。

mori · 2007-05-31 17:50

おぉう。 見間違いだった、実行可能ですね。 HAHAHA。

訂正ついでに。

2つのfor文の内側のコードは、配列の要素のうち辞書順で最も後ろのもの(もしくは前のもの)を配列の最後の要素に移動させるコードになっています。
2回目以降、最後の要素まで比較を行うのは、compare()/compareTo()の返り値が必ず負になるため、比較の無駄になります。 

てことで、だんだん内側のループを行う範囲を狭くすると、2倍くらい効率がいいです。

squld · 2007-05-31 19:04

僕が書くならこんな感じかなぁ。

import java.util.Comparator;

public class Sorter<ElementType> {
  public static void main(String[] aArguments) {
    System.out.println("元ネタ");
    display(aArguments);

    Sorter<String> tSorter = new Sorter<String>();

    System.out.println("並び替えます(正順)");
    tSorter.bubbleSort(aArguments, new Comparator<String>() {
      public int compare(String aLeft, String aRight) {
        return aLeft.compareTo(aRight);
      }
    });
    display(aArguments);

    System.out.println("並び替えます(逆順)");
    tSorter.bubbleSort(aArguments, new Comparator<String>() {
      public int compare(String aLeft, String aRight) {
        return aRight.compareTo(aLeft);
      }
    });
    display(aArguments);
  }

  private static void display(String[] aArguments) {
    for (String aArgument : aArguments) {
      System.out.print(aArgument + " ");
    }
    System.out.println();
  }

  public void bubbleSort(ElementType[] aTarget, Comparator<ElementType> aComparator) {
    int tLoopLength = aTarget.length - 1;
    for (int k = 0; k < tLoopLength; k++) {
      for (int i = 0; i < tLoopLength; i++) {
        if (aComparator.compare(aTarget[i], aTarget[i + 1]) > 0) {
          ElementType w = aTarget[i];
          aTarget[i] = aTarget[i + 1];
          aTarget[i + 1] = w;
        }
      }
    }
  }
}

変更点は以下の通りです。

  • 変数にはプレフィックスを付ける。 “a” → 引数, “t” → ローカル変数, “m” → メンバ変数
  • 変数名は略さず、意味が分かりやすいものにする
    • args → aArguments 等
  • 同じ処理はメソッドにまとめる
    • for(String s:args)System.out.print(s+” “); → display(aArguments)
  • ソートは汎用的なものなので、型を縛らないようにジェネリクスを使う。
    • String → ElementType
  • 使い捨てコンパレータクラスは無名インナークラスにする
    • MyClass, ReMyClass等
  • 不要なメソッド・変数は消す
    • public static Object sort(String[] args){
  • ループ内での計算量を抑える
    • for(int k=0; k < args.length - 1; k++) → for (int k = 0; k < tLoopLength; k++) 意外ですが、args.length - 1はループ中毎回計算されます。

アルゴリズムの改善については、mori御大に聞いてくださいw

squld · 2007-05-31 23:48

課題を考えました。

  1. 配列を与えて、配列を取り出すフィルタメソッドを作る。条件はConditionインタフェースとして受け取る。
    • public ElementType[] subArray(ElementType[] aTargets, Condition<ElementType> aCondition)
  2. Condition型は判定メソッドを持つ。
    • public bool judge(ElementType aTarget)
  3. Comparator<ElementType>により順序定義された要素に対して、ある値よりも大きな要素を判定する、GreaterCondition<ElementType>クラスを定義する。このクラスのコンストラクタにはComparator<ElementType>と閾値となる要素を与える。
    • public GratorCondition(Comparator<ElementType> aComparator, ElementType aThresholdValue)
  4. Comparator<ElementType>により順序定義された要素に対して、ある値と等しい要素を判定する、EqualCondition<ElementType>クラスを定義する。このクラスのコンストラクタにはComparator<ElementType>と判定対象となる要素を与える。
    • public EqualCondition(Comparator<ElementType> aComparator, ElementType aValue)
  5. Conditionを2つ持ち、両方のjudge結果がtrueだった場合にtrueを返すjudgeメソッドを持つAndConditionを作る。
    • public AndCondition(Comparator<ElementType> aLeftComparator, Comparator<ElementType> aRightComparator)

  6. Conditionを2つ持ち、いずれかのjudge結果がtrueだった場合にtrueを返すjudgeメソッドを持つOrConditionを作る。
    • public OrCondition(Comparator<ElementType> aLeftComparator, Comparator<ElementType> aRightComparator)

  7. 出来上がった、subArrayメソッドと、Conditionで色々遊ぶ。
    • Conditionをインタフェースにすることによって何が嬉しいか?
    • Compositパターンとは何か?今回の課題とどう関係があるか?
    • ジェネリクスとは何か?ジェネリクスを使うことによって何が嬉しいか?
    • 配列とListの用途の違いを理解する。また、使い分けの指針は?

結果はこんな感じになるはず。

{ "d", "e" } ← subArray(new String[] { "a", "b", "c", "d", "e" }, new GratorCondition<String>(new AscendingComparator(), "c"))
{ "a", "b", "c", "d" } ← subArray(new String[] { "a", "b", "c", "d", "e" }, new GratorCondition<String>(new DescendingComparator(), "e"))
{ "d" } ← subArray(new String[] { "a", "b", "c", "d", "e" },
    new AndCondition(
      new GratorCondition<String>(new AscendingComparator(), "c"),
      new GratorCondition<String>(new DescendingComparator(), "e")
    )
  )
{ "a", "b", "c", "d", "e" } ← subArray(new String[] { "a", "b", "c", "d", "e" },
    new OrCondition(
      new GratorCondition<String>(new AscendingComparator(), "c"),
      new GratorCondition<String>(new DescendingComparator(), "e")
    )
  )

koreyasu · 2007-06-02 16:08

ソースを整形しておきました。
ちょっと見るに耐えないぐらいぐちゃぐちゃだったので^^;

koreyasu · 2007-06-03 13:34

squldさんが、改変しつつ指摘してない点を。

import java.util.*;

importは*でやるのは止めましょう。*の場合は配下に存在するクラスをすべてロードしてしまいます。
必要なクラスのみを一つずつimportするのがよいです。
Eclipse上だと、Ctrl+Shift+oで必要なクラスのインポートを行ってくれます。

現在コメントは受け付けていません。