Why does this randomizer sort function fail sometimes?

[Bugala]

I was planning on making Randomizer function using Sort command, and did following:

Code: Select all

Table = {1, 2, 3, 4, 5, 6, 7, 8}

Sort(Table, Function(a, b) 
	Local Rando=Rnd(2)
	If Rando = 0 
		Return(a > b)
	Else
		Return(a < b)
	EndIf
EndFunction)

ForEach(Table, DebugPrint)

It works nicely when it works, but every now and then, like maybe every 5 executions, it gives me an error of:
"Error in line 3 (Unnamed5): Invalid order function for sorting!"

But why?

Also, for a wishlist feature, A "Sort(Table, Random)" would be handy sometimes.

[Flinx]

I don't know which algorithm is used by Sort – Quicksort or something like that – but changing the return value inconsistently will collide with it. Your function sometimes returns different results for the same numbers, which cannot go well when sorting.
For such things I use Fisher-Yates shuffle.

[jPV]

https://forums.hollywood-mal.com/viewtopic.php?p=12119

[Bugala]

@JPV

Hmm. Funny thing. Had no memory I had actually answered someone regarding this before

[jPV]

Yeah, and your function returns 0 or 1, so it could be optimized to

Code: Select all

Sort(Table, Function() Return(Rnd(2)) EndFunction)

But as said, it still doesn't work

[Flinx]

That's one of my trials with Fisher-Yates:

Code: Select all

Function shuffle(li)
	For ix=ListItems(li)-1  To 1 Step -1
		Local r=Rnd(ix+1)
		Local temp=li[r]
		li[r]=li[ix]
		li[ix]=temp
	Next
EndFunction

li={1,2,3,4,5,6,7,8}
shuffle(li)
For i=0 To ListItems(li)-1
	DebugPrint(li[i])
Next

It works with subtables too.

[jPV]

I would change three things in it:
1) Use Local iterator variables, otherwise ix and i become global and may cause troubles or slowdowns
2) Temp variable isn't needed as you can swap variable contents with one line too
3) Hollywood styleguide instructs to name own functions with the "p_" prefix

Here's with my suggestions:

Code: Select all

Function p_shuffle(li)
	For Local ix=ListItems(li)-1 To 1 Step -1
		Local r=Rnd(ix+1)
		li[r],li[ix]=li[ix],li[r]
	Next
EndFunction

li={1,2,3,4,5,6,7,8}
p_shuffle(li)
For Local i=0 To ListItems(li)-1
	DebugPrint(li[i])
Next

[Flinx]

Thank you, all makes sense. (In my real project I did it this way, except for the swapping, which I didn't know yet.)