Problem with QuickSort algorithm in C#

Question

i wrote i quicksort algorithm in c# but it has a problem,when i compile it,it doesnt work in some conditions for example when i enter number 12,32,11 in textbox6 to sort, it gives out of range error when i go to trace it, debugger shows that num[] took nums[0]=12,nums[1]=11,nums[2]=1

Edited: i changed the while condition and it know doesnt give out of range error but when the input is 12,32,11 output 11,12,1

private void button5_Click(object sender, EventArgs e)
    {
        string[] x = textBox6.Text.Split(',');
        int[] nums = new int[x.Length];

        for (int counter = 0; counter < x.Length; counter++)
        {
            nums[counter] = Convert.ToInt32(x[counter]);
        }

        int i = 0;
        int  j = nums.Length;
        int pivot = nums[0];
        do 
        {
            do 
            {
                    i++;
            }
                  while ((i < nums.Length) && (nums[i] < pivot));
            do
                {
                    j--;
                }
while (nums[j]>pivot);
            if (i < j)
            {
                int temp = i;

                nums[i] = nums[j];
                nums[j] = temp;

            }

        }while(i<j);
        int temp1 = nums[0];
        nums[0] = nums[j];
        nums[j] = temp1;
        int pivotpoint = j;
        string QuickSort = "";
        foreach (var n in nums)
           QuickSort += n.ToString() + ",";
        textBox5.Text = QuickSort;

    }

perhaps you should post the complete code... Anyway, implementing a quick sort in a button event handler doesn't seem like a very good idea... — Thomas Levesque
– Thomas Levesque, Commented Dec 19, 2010 at 11:22
1) You didn't post your complete loop. 2) I see no recursion. So how is this supposed to be quick-sort? — CodesInChaos
– CodesInChaos, Commented Dec 19, 2010 at 11:27
@Thomas: This is most likely a student's code, and apparently an assignment, so the event handler is probably the best place in such a case :) (although it should certainly be refactored). — Hosam Aly
– Hosam Aly, Commented Dec 19, 2010 at 11:34
First things first, is there a problem in the Quicksort or already in the decoding of the Textbox text? — Henk Holterman
– Henk Holterman, Commented Dec 19, 2010 at 12:12

Lazarus · Accepted Answer · 2010-12-19 11:27:26Z

2

Your issue is that i isn't being bounds-checked in your do-while loop. i will get incremented beyond the end of your nums array. Try modifying the while condition thus:

while ((i<nums.Length) && (nums[i]<pivot))

Of course you could simply use Array.Sort(nums);

edited Dec 19, 2010 at 11:27

answered Dec 19, 2010 at 11:18

Lazarus

43.2k4 gold badges47 silver badges54 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

Hosam Aly Over a year ago

This is most likely a student's code, trying to learn how to implement the quicksort algorithm, so he probably shouldn't be using Array.Sort.

Lazarus Over a year ago

@Hosam Aly: As we aren't allowed to tag questions as homework any more, I have to consider any question as non-homework. Either way, my purpose here is to answer the question not to ensure that their homework assignment is completed correctly. What is ironic is that the question was edited after my answer and still the OP has two while conditionals with no bounds checking.

Arash Over a year ago

@Lazarus:Excuse me sir,but i just completed my codes and didnt do anything else.i didnt change my code as you said because i thaught your answer may change when you see the complete code

Lazarus Over a year ago

@arash: Fair enough. Have you tried adding bounds checking to the while loops?

Arash Over a year ago

@Lazarus:no,because i didnt understand completly where should i change, im a collegian and i m writing this code to understand it,is this crime?

|

Collectives™ on Stack Overflow

Problem with QuickSort algorithm in C#

1 Answer 1

9 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

9 Comments

Your Answer

Sign up or log in

Post as a guest

Related