2010/04/01

Would you like to help C# programming?

C#が得意という方。
ぜひ教えてほしいことがあります。
授業で書いたプログラムなのですが、このままコンパイルするとコメントの☆部分が原因でソートできずにランタイムエラーになっちゃいます。
なぜエラーになるのか、もし分かる方がいたら教えてください。
いろいろ調べたものの見つからず。。。。。
もし分かる方がいたら教えてくださいー。
よろしくお願いします!


プログラムの概要
  1. 3つのクラスで構成
  2. Employeeクラス
  • EmployeeID、Name、Salary の変数
  • Salaryが年5%増加と仮定してGetNewSalary()メソッドで新しいサラリーを計算
3. Sortingクラス
  •  IComparerのinheritanceを利用
  • ランタイムエラーのコメントは以下のとおりです。
  • ハンドルされていない例外: System.ArgumentException: Array.Sort が x. CompareTo(x) を呼び出したときに、IComparer または依存する IComparable メソッドは 0 を返しませんでした。x: 'ConsoleApplication24.Employee'  x の型: 'Employee' IComparer: 'ConsoleApplication24.Sorting'
4. Programクラス
  • AnnualIncrementを入力してもらい、Valueを決定
  • Employeeオブジェクトを4つ作成しArrayListにインプット
  • GetNewSalaryで各自の新しいサラリーを表示(ここまでは表示できます。)
  • 新しいサラリーを昇順でソートしてアウトプット(ここでエラー)


namespace ConsoleApplication24
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Please input increment");
            string j = Console.ReadLine();
            double inc = double.Parse(j);
            Employee.SetIncrement(inc);

            ArrayList al = new ArrayList();
            al.Add(new Employee(1, "ABC", 200));
            al.Add(new Employee(2, "DEF", 100));
            al.Add(new Employee(3, "GHK", 300));
            al.Add(new Employee(4, "OPQ", 250));

            foreach (Employee n in al)
            {
                Console.WriteLine(n.GetNewSalary());
            } 
try
            {

            al.Sort(new Sorting());
foreach (Employee m in al)
                {
                    Console.WriteLine(m.GetEID() + " " + m.GetEName() + " " + m.GetNewSalary());

                }
            }
            catch
            {
                Console.WriteLine("Try block error");
            }
        }

    }
    class Employee
    {
        int EID;
        string EName;
        double Salary;
        public static double Increment = 0;

        static public void SetIncrement(double d)
        {
            Increment = d;
        }

        public int GetEID()
        {
            return EID;
        }
        public string GetEName()
        {
            return EName;
        }
        public double GetNewSalary()
        {
            this.Salary = Salary + (Salary * Increment);
            return this.Salary;
            //☆この部分がエラー。下の2行に置き換えると問題なく動く。
            //double NewSalary = Salary + (Salary * Increment);
            //return NewSalary;
        }
        public Employee(int id, string name, double salary)
        {
            EID = id;
            EName = name;
            Salary = salary;
        }
    }
    class Sorting : IComparer
    {
        public int Compare(Object a, Object b)
        {
            Employee em = (Employee)a;
            Employee em1 = (Employee)b;
            double sal = em.GetNewSalary();
            double sal1 = em1.GetNewSalary();
            int value = 0;


            if (sal > sal1)
            {
                value = 10;
            }
            else if (sal == sal1)
            {
                value = 0;
            }
            else if (sal < sal1)
            {
                value = -10;
            }

            return value;
        }
    }
}

8 件のコメント:

  1. Employee のコンストラクタで
    Salary = salary; としてますが、
    this.Salary = salary; ではありませんか?

    返信削除
  2. アドバイスありがとうございます。
    this.Salary = salary;でもエラーでした><
    SortingクラスがGetNewSalaryを取得する際に計算前のSalaryと計算後のSalaryでソート順に矛盾がおきてしまうからではないかというアドバイスを別の方からもらいました。
    GetNewSalaryメソッドを二つに分割し、1個はNewSalaryの計算でGetNewSaraly内では計算をせずにreturn Salary;のみにしたところ動きました。
    なんとなくロジックは理解できるものの、完全な理解ではないので、もしお分かりの方がいれば引き続き教えていただけると助かります!
    よろしくお願いします。

    返信削除
  3. エラーの原因としては上記にあるとおりだと思います。

    ただ、 GetNewSalary() の実装は意図した仕様とは異なる動作をしていると思います。
    GetNewSalary()の動作を日本語で説明すると、
    1.メンバ変数Salaryに(Salary*Increment)を加算する。
    2.その値をreturnする。
    となっており、このメソッドがコールされる度にメンバ変数Salaryが増加します。

    では、このメソッドがコールされるタイミングを考えてみましょう。

    まず、 Program.Main() の中の、
    Console.WriteLine(n.GetNewSalary());
    で1回実行されます。

    次に、 al.Sort(new Sorting() ); の内部で、
    Sorting.Compare() が複数回実行されます。(後述)
    Compare() の内部には
    double sal = em.GetNewSalary();
    double sal1 = em1.GetNewSalary();
    があるのでまた GetNewSalary() が複数回実行されます。

    さらに、戻って Program.Main() の、
    Console.WriteLine(m.GetEID() + " " + m.GetEName() + " " + m.GetNewSalary());
    で1回実行されます。

    結果、 GetNewSalary() が複数回実行されるため、Salaryが複数回に渡って加算されています。
    これは意図した仕様と異なっているのではないかと思います。

    返信削除
  4. 「Salaryを加算するメソッド」と「現在のSalaryを返却するメソッド」の2つに分けるのが正解です。

    「Salaryを加算するメソッド」は単純に加算するだけです。
    public void RaiseSalary(){
    Salary += Salary * Increment;
    }

    「現在のSalaryを返却するメソッド」は単純にSalaryを返却するだけです。
    public double GetSalary(){
    return Salary;
    }

    こうすることで、
    昇給させたいときは、 RaiseSalary() をコールするだけ、
    現在の給料を取得したいときは、 GetSalary() をコールするだけ、
    となり、シンプルになります。シンプルであることは重要です。

    返信削除
  5. さて、後述するとなっていた、「 ArrayList.Sort() がコールされると、Sorting.Compare() が複数回実行される」について説明します。

    Compare()はあくまで「2つのEmployeeのインスタンスの大小を決定する基準」が実装されていると思います。
    ArrayList.Sort() は内部でその「大小を決定する基準」を使ってソートしています。
    (上記のプログラムを見る限り、それについては理解されていると思います)
    ArrayListの中から2つずつ取り出して、どちらが大きい?という比較を繰り返して順序を決めてソートしています。

    ですので現状の実装では以下の2つの問題が発生しています。
    1.Compare()が複数回コールされ、結果 GetNewSalary() が複数回コールされたため、Salaryが想定よりも増えている。
    2.Salaryのソート中にSalaryの値が変更されているため、正常にソートすることができずエラーとなっている。

    先ほどの説明のとおり、2つのメソッドに分割し、 Compare() の中では、
    「現在のSalaryを返却するメソッド」のみコールさせることで値がソート中に増加してしまうことを防げます。

    あえてエラーメッセージを説明するとすれば、
    「Array.Sort が x. CompareTo(x) を呼び出したときに、IComparer または依存する IComparable メソッドは 0 を返しませんでした。」
        ↓
    「xとxを比較(Compare)したら異なる値だった(0を返さなかった)ため、比較しようがありませんでした。」
    というようなことになります。


    長くなっちゃいました。C#を開発できる環境を持っていないので、誤っていたらすみません。
    楽しんで学習されているのは素敵ですね。是非がんばってください。

    返信削除
  6. Bigmacさん

    どうも、ker0r0です。
    もう解決されたでしょうか?

    私の見解ですが、IComparerのCompareメソッドは、「引数のObject aとObject bが全く同じものの場合、0を返さなければならない」という制約があったと思います。

    ところが、GetNewSalary()内で

    this.Salary = Salary + (Salary * Increment);

    と、フィールドの値を書き換えていると、emとem1が全く同じObjectの場合でも、salとsal1は違う値になります。

    double sal = em.GetNewSalary(); //←ここで書き換わる
    double sal1 = em1.GetNewSalary();

    とりあえずは、Employee em1 = (Employee)b;の直後に、
    if (em == em1){return 0;}
    と入れてやれば動きます。

    (おそらく想定されてたのは、☆の下のコメント部の動きだと思いますが…)

    それではまた。

    返信削除
  7. Anonymousさん
    丁寧な解説ありがとうございます。
    ようやくなぜエラーになるのかロジックが理解できました。
    いまさらながらネットってすばらしいですね^^
    こんなに丁寧におしえてくれる人がいるなんて、びっくりです。
    今後は用意されているインタフェースを利用する際は仕様どおりの使い方を意識するようにします。
    Object Oriented Programmingのコンセプトがようやく理解できた段階なのでこれからですがC#はとっても楽しいです。これを考えた人は本当にスマートですね。
    またお世話になると思うので、どうぞよろしくお願いします!

    返信削除
  8. ker0r0さん
    アドバイスありがとうございます。
    Employee em1 = (Employee)b;の直後に、
    if (em == em1){return 0;}いれてみました。
    動きましたが、アウトプットの計算が変なことになっちゃいました^^
    やはり意図してない使い方のせいですね。
    -------アウトプット-------

    Please input increment
    0.2
    240
    120
    360
    300
    2 DEF 429.981696
    3 GHK 895.7952
    4 OPQ 1074.95424
    1 ABC 1238.34728448
    続行するには何かキーを押してください . . .

    Incrementが0.2なのにすごく端数がでちゃいました^^;
    Compare()メソッドから呼ぶGetNewSalary内では計算せずただNewSalaryを返すだけにします。
    どうもありがとうございますー!!
    今週末にNZジョブサーチ結果アップしようと思ってますので、よかったら見てください。ではでは、これからもよろしくお願いします。

    返信削除

UA-9417263-1