leopard-js / leopard

Library for making Scratch-like projects with JavaScript.
https://leopardjs.com
MIT License
138 stars 26 forks source link

Should "delete all of list" be a mutating splice() instead of overwriting the property? #165

Open towerofnix opened 1 year ago

towerofnix commented 1 year ago

(This is technically an sb-edit change, but more pertinent to Leopard.)

Scratch doesn't support first-class lists, but JavaScript does, and people digging deeper into Leopard may be interested in treating their own lists as first-class citizens.

Currently, the "delete all of list" block is non-mutating: it replaces the value in the variable with a completely new (empty) list. That means any alternate references to the list will not have their contents updated, which may be a bit of a surprise, since Scratch certainly makes "delete all" seem like a mutating operator! (You don't need to re-show a list watcher when the list it's referencing has its contents cleared, for example.)

The alternative is replacing this.vars.myList = [] syntax with this.vars.myList.splice(0, this.vars.myList.length). This could be moved into a this.emptyArray(this.vars.myList) function, but it's more of a Javascript-ism than a Leopard-ism, so I think it's OK to leave this common albeit mildly clunky syntax (it's another real-world example of splice which is already exposed through "replace item" and "insert item").

adroitwhiz commented 1 year ago

this.vars.myList.length = 0 works too; not sure how it compares in terms of clarity.

towerofnix commented 1 year ago

Terrifying. I see you have coded JavaScript in the real world.

I have a preference for splice for consistency with other list ops and showing a real-world use, but length = 0 is significantly more compact and functionally equivalent (without introducing a Leopard-ism), so I could go either way.