sjbarag / brs

An interpreter for the BrightScript language that runs on non-Roku platforms.
MIT License
114 stars 43 forks source link

Fixes many sorting issues with mixed arrays (fixes #591) #592

Closed markwpearce closed 3 years ago

markwpearce commented 3 years ago

Refactors roArray sort() and sortBy() so it more closely matches what Roku actually does.

Fixes #591

array.sort() example:

Brightscript Debugger> 
arr=["a", 1, ["sub", "array"], 9, 10, "B", {obj: "foo"}, {obj: "bar"}, false, "c", true, false]

Brightscript Debugger> 
arr.sort()

Brightscript Debugger> 
?arr
<Component: roArray> =
[
    1
    9
    10
    "B"
    "a"
    "c"
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roArray>
    false
    true
    false
]

Brightscript Debugger> 
?arr[6]
<Component: roAssociativeArray> =
{
    obj: "foo"
}

Brightscript Debugger> 
?arr[7]
<Component: roAssociativeArray> =
{
    obj: "bar"
}

Brightscript Debugger> 

**Mixed Arrays seem to get sorted in this order (tested with Roku OS 9.4):

  1. numbers in numeric order
  2. strings in alphabetical order,
  3. assocArrays in original order
  4. everything else in original order**

....

here's another example:

Brightscript Debugger> 
arr = [{bar:"def"}, {bar:"abc"},{foo: "bar"}, {foo:"abc"},3,2,1,"def", "abc", false, true, false, ["sub","array"], {foo: "buz"}, false]

Brightscript Debugger> 
arr.sort():?arr

<Component: roArray> =
[
    1
    2
    3
    "abc"
    "def"
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    false
    true
    false
    <Component: roArray>
    false
]

Brightscript Debugger> 
?arr[5];arr[6]arr[7];arr[8]
<Component: roAssociativeArray> =
{
    bar: "def"
}<Component: roAssociativeArray> =
{
    bar: "abc"
}<Component: roAssociativeArray> =
{
    foo: "bar"
}<Component: roAssociativeArray> =
{
    foo: "abc"
}

SortBy is a little different

it goes :

  1. numbers in numeric order
  2. strings in alphabetical order,
  3. assocArrays
    1. any assoc array with the key, sorted by the value with that key
    2. any other assocArray, in original order
  4. everything else in original order

Here's a sortBy() example ---

Brightscript Debugger> 
arr = [{bar:"def"}, {bar:"abc"},{foo: "bar"}, {foo:"abc"},3,2,1,"def", "abc", false, true, false, ["sub","array"], {foo: "buz"}, false]

Brightscript Debugger> 
arr.sortBy("foo"):?arr
<Component: roArray> =
[
    3
    2
    1
    "def"
    "abc"
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    <Component: roAssociativeArray>
    false
    true
    false
    <Component: roArray>
    false
]

Brightscript Debugger>
?arr[5];arr[6]arr[7];arr[8];arr[9]
<Component: roAssociativeArray> =
{
    foo: "abc"
}<Component: roAssociativeArray> =
{
    foo: "bar"
}<Component: roAssociativeArray> =
{
    foo: "buz"
}<Component: roAssociativeArray> =
{
    bar: "def"
}<Component: roAssociativeArray> =
{
    bar: "abc"
}
markwpearce commented 3 years ago

I updated this PR after doing some more testing. I think my updates are actually much more readable, too.

markwpearce commented 3 years ago

@sjbarag - Updated with some fixes you requested.

Any chance you might also review https://github.com/sjbarag/brs/pull/583 ?

sjbarag commented 3 years ago

@sjbarag - Updated with some fixes you requested.

Any chance you might also review #583 ?

Of course! Sorry for the delay — this is a bit of a "side project at work" that I'd love to spend more time on. Checking that out today!