2

Is there anyway to make a quicksort sort by multiple conditions? For example, I have a set of edges. Each edge has a source, destination, and length. I want to put the edge with a smaller length in my array first. But if the lengths are the same, I want to sort by that with a smaller source vertex. If these source vertexes are the same, I want to sort by the smaller of the two destination vertices.

For example:

4 (source) 2 (destination) 3 (length)

1 (source) 5 (destination) 3 (length)

Since they both have the same length, we look at the source vertex. Since the second edge is smaller than the first edge, we swap them because we compare by source vertex.

Below is my quicksort and I'm honestly not sure why it's not sorting correctly.If there's a way to make quicksort less efficient but more stable, I would gladly take suggestions!

void quickSort(edge *e, int left, int right)
{
  int i = left, j = right;
  int temp, temp1, temp2;
  int pivot = (left + right)/2;
  while(i <= j)
  {
    while(e[i] < e[pivot])
      i++;
    while(e[pivot] < e[j])
      j--;
    if(i <= j)
    {
      temp = e[i].getLength();
      temp1 = e[i].getEdgeSrc();
      temp2 = e[i].getEdgeDes();
      e[i].setLength(e[j].getLength());
      e[i].setEdgeSrc(e[j].getEdgeSrc());
      e[i].setEdgeDes(e[j].getEdgeDes());
      e[j].setLength(temp);
      e[j].setEdgeSrc(temp1);
      e[j].setEdgeDes(temp2);
      i++;
      j--;
    } //if statement
  }///while loop
  if(left < j)
    quickSort(e, left, j);
  if(i < right)
    quickSort(e, i, right);
}

My sorting of conditions:

bool edge::operator<(const edge &other) const 
{
    if (length < other.length)
        return true;
     else if ((length == other.length) && (source < other.source))
        return true;
     else if((length == other.length) && (source == other.source) && (destination < other.destination))
        return true;
     return false;
}

Again, if anyone knows a way to make this quicksort correctly by reducing the time complexity of it but making it stable, I would gladly take any suggestions! Thank you! Any help?

Edit: This is how I invoked my quicksort. I invoked it based on the number of edges read.

    quickSort(e, 0, edges-1); //-1 because if you put in edges, it'd go past the bounds of the array

EDIT: when I try to put in something like this in my algorithm:

0 1 1

0 3 1

1 3 1

2 5 1

4 10 1

4 8 1

10 8 1

11 6 2

11 7 2

6 7 1

9 6 1

9 7 1

This is the output:

0 1 1

0 3 1

1 3 1

2 5 1

4 8 1

4 10 1

6 7 1

6 9 1

8 10 1 <- should be below 7 9 1

7 9 1 <- should be above 8 10 1

6 11 2

7 11 2

user200081
  • 563
  • 2
  • 12
  • 24
  • 1
    Quicksort is not a naturally stable sort (it can be forced to be so, but...). Why not use a sort that is naturally stable? – dmckee --- ex-moderator kitten Nov 19 '12 at 01:19
  • Could you tell me of a way to force it to be stable with regards to my conditions? I just wanted to learn how to use Quicksort. – user200081 Nov 19 '12 at 01:24
  • You have to record the original ordering somehow and use that to resolve ties. It is rarely worthwhile. – dmckee --- ex-moderator kitten Nov 19 '12 at 01:26
  • 1
    You're guess of not finishing sorted due to quicksort not being stable is not correct. Stable only means the items maintain natural sort order. If your data is not sorted when finished, it is *not* because quicksort isn't stable. It is a bug either in your algorithm or the method of invoking it. – WhozCraig Nov 19 '12 at 01:30
  • Hm, @WhozCraig do you see any flaws in my quicksort that may make it wrong then? I added in how I invoked quicksort. – user200081 Nov 19 '12 at 01:32
  • is your output code correct? I think you are flipping Src and Dest in output(or input) – Karthik T Nov 19 '12 at 01:40
  • My output is correct. I simply printed htem right after the quicksort. – user200081 Nov 19 '12 at 01:43
  • `11 6 2` -> `6 11 2` , `9 7 1` -> `7 9 1`, etc, etc – Karthik T Nov 19 '12 at 01:45
  • Hm, there's another input that I found outputs incorrectly. Might it be because of the get/set functions? are they error prone? – user200081 Nov 19 '12 at 01:47
  • I would advice against using those, if only to make code more readable. go for `temp = e[i]` etc and `e[i].print()` style of code. Since your members are all simple datatypes you dont need to write a copy constructor – Karthik T Nov 19 '12 at 01:54
  • Well I use get and set functions due to the fact that my members are private. – user200081 Nov 19 '12 at 01:57
  • Members can be private thats fine(indeed recommended). What im suggesting is to make a member function called print which does the printing. That way it is easier to avoid issues like the above printing bug. Also you can try the following, you will see it works without any issues. `edge temp; ...; temp = e[i]; e[i] = e[j]; e[j] = temp;` – Karthik T Nov 19 '12 at 02:01
  • Did you use a reference to implement this version of quicksort? If so can you link it so we can compare? – Karthik T Nov 19 '12 at 02:06
  • I used http://www.algolist.net/Algorithms/Sorting/Quicksort – user200081 Nov 19 '12 at 02:08
  • Can you change your pivot to an `edge` value instead of an index and see if it helps, thats what the link you provide does – Karthik T Nov 19 '12 at 02:25

2 Answers2

1

It is cleaner to write it this way

if (length != other.length)
   return length<other.length;

if ( source != other.source)
   return source < other.source;

return destination < other.destination;

You should also be able to do temp = e[i] and so on since the members are all ints.

This (and the code you submitted) should do the task you want I think.

If you are having stability issues, thats because quicksort isnt stable. You could get around it by adding more conditions so that lhs==rhs doesnt happen. Alternatively you can try Mergesort

I dont have much experience with Quick sort frankly, but your impl does look markedly different from Wikipedias In Place Algorithm. For instance, your pivot is not moved at all. Could you check if that is the problem?


Edit

After looking at your link

It looks like the algorithm linked also uses pivot as a value instead of as an index (as you do). It looks syntactically identical to yours until you consider that your pivot value might move, after which your pivot index would point to something else

int pivot = arr[(left + right) / 2];

Does this help?

Community
  • 1
  • 1
Karthik T
  • 31,456
  • 5
  • 68
  • 87
  • What you are having is NOT a stability issue afaik. Can you try if my code segment changes the output? – Karthik T Nov 19 '12 at 01:38
  • Hm yeah I realized it's not stability. I tried your code and it didn't change the output... it's like the example i posted is an example of how the code is screwing up. But weirdly enough, it only does it for certain outputs--not all – user200081 Nov 19 '12 at 01:43
  • I acutally fixed it. I simply stored it into a group of smaller arrays and sorted the smaller arrays and it worked fine. Thank you for your patience and help, I really appreciate it! – user200081 Nov 19 '12 at 02:30
  • lolz. "Does this help?" After all the talk of stable vs. not, etc. That is **the** answer. I wish I could up-vote it more than once. I sure hope he sees this, because this will likely solve his problem entirely. Compared to the quicksort I fixed for [another question](http://stackoverflow.com/questions/13373525/sorting-an-array-using-multiple-sort-criteria-quicksort/13374398#13374398) you'd think I would have spotted that error. The one I fixed for the other answer uses l,r,p all for offset indexes, so thats probably why I never gave it a second look. Anyway, hope he sees this. – WhozCraig Nov 19 '12 at 03:35
  • Thanks, just curious tho, the answer you refer to, doesnt that have the same problem? – Karthik T Nov 19 '12 at 04:54
0

EDIT: Here's pseudocode for in-place quicksort: http://en.wikipedia.org/wiki/Quicksort#In-place_version

This differs from your code in that the pivot is a value (an average of the left and right values) rather than an index.

If you're looking for a simple non-optimal solution, mergesort the entire list by destination vertex, then mergesort the entire list by origin vertex, then mergesort the entire list by edge length. This takes advantage of the fact that mergesort is a stable sort algorithm and has running time O(E) on the number of edges.

Daniel
  • 855
  • 10
  • 21
  • Would inplace quicksort solve this issue though? just curious. – user200081 Nov 19 '12 at 01:56
  • It looks like your code has a subtle bug (no idea what it is), due to which it is not sorting. Thus InPlace quick sort (or really any sort) should work. – Karthik T Nov 19 '12 at 02:05
  • This issue is that your algorithm isn't quicksort so it's not guaranteed to work. To make your algorithm quicksort, pivot needs to be an `edge`, not an `int`. – Daniel Nov 19 '12 at 02:21