Bubble sort when using struct members

Question

struct SSales
    {
        private int Y;
        private double S;

        public int Year
        {
            get { return Y; } 
            set { Y = value; }

        }
        public double Sale
        {
            get { return S; }
            set { S = value; }
        }

        public SSales (int _year, double _sales)
        {

            Y = _year;
            S = _sales;

        }

private void Sortbutton_Click(object sender, EventArgs e)
    {
        listBox1.Items.Clear();
        if (yearradio.Checked)
        {
            int temp = 0;
            for (int i = 0; i < bubble.Length - 1; i++)
            {
                for (int j = 0; j < bubble.Length - 1; j++)
                {
                    if (bubble[i + 1].Year < bubble[i].Year)
                    {
                        temp = bubble[i].Year;
                        bubble[i].Year = bubble[i + 1].Year;
                        bubble[i + 1].Year = temp;
                    }
                }
            }

        }
        if (salesradio.Checked)
        {
            double temp2 = 0;
            for (int i = 0; i < bubble.Length - 1; i++)
            {
                for (int j = 0; j < bubble.Length - 1; j++)
                {
                    if (bubble[i + 1].Sale > bubble[i].Sale)
                    {
                        temp2 = bubble[i].Sale;
                        bubble[i].Sale = bubble[i + 1].Sale;
                        bubble[i + 1].Sale = temp2;

                    }
                }
            }
        }
        for (int i = 0; i < bubble.Length; i++)
        {

            listBox1.Items.Add(bubble[i].ToString());

        }

    }

While my bubble sorting algorithms work perfectly fine, they only sort incrementally with each click of the Sort Button. I need the listbox to be completely sorted with 1 click.

Also, as my code is now, the Years and Sales reorganize completely independent of each other. When the Sales index changes, the corresponding Year index remains at the same location and vice versa.

I'm guessing a for loop with int j would work, but I'm not sure how to implement it. Any help would be appreciated!

Answer 1

I will guess that you are doing bubble sort for learning / practice reasons. If not you should just use the inbuilt Array.Sort() or Enumerable.OrderBy() or something similar.

There are multiple things you have done wrong. I have the improved code below which I will explain

struct SSales {
    public int Year { get; set; } // use auto-properties for brevity

    public double Sale { get; set; }  // use auto-properties for brevity

    public SSales(int year, double sales) {
        Year = year;
        Sale = sales;
    }
}

// Use a generic routine to Swap, instead of replicating the code multiple times
// Note that we are passing by reference so the actual array eventually gets sorted
// Also, don't swap the properties, but the whole record. Else it will corrupt your data
static void Swap<T>(ref T obj1, ref T obj2) {
    var temp = obj1;
    obj1 = obj2;
    obj2 = temp;
}

// Write the sort routine separately. Sorts usually just need a way to compare records, which can be provided by Caller (IoC pattern)
static void Sort<T>(T[] items, Func<T, T, int> comparer) {
    for (int i = 0; i < items.Length - 1; i++) {
        // Every execution of the inner loop will bubble-up the largest element in the range
        // Your array is getting sorted from the end, so you don't need to re-compare the already sorted part
        for (int j = 0; j < items.Length - 1 - i; j++) {
            if (comparer(items[j], items[j + 1]) > 0) // call the generic user provided comparer to know the sequence
                Swap(ref items[j], ref items[j + 1]); // use teh generic swapper to swap elements in the array
        }
    }
}


private void Sortbutton_Click(object sender, EventArgs e) {
    listBox1.Items.Clear();

    if (yearradio.Checked) {
        // Invoke the Sort routine, with the required comparer
        Sort(bubble, (a, b) => a.Year - b.Year);
    }

    if (salesradio.Checked) {
        // Invoke the Sort routine, with the required comparer
        Sort(bubble, (a, b) => (int)(a.Sale - b.Sale));
    }

    for (int i = 0; i < bubble.Length; i++) {
        listBox1.Items.Add(bubble[i].ToString());
    }

}

Hope that clarifies the issues you are facing and also helps you learn how to write better C# code.