[BUG]Associative Arrays Topic is solved

Discuss the future of the AutoHotkey language
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

[BUG]Associative Arrays

02 Jan 2018, 13:47

AutoHotkey_2.0-a081-cad307c

Code: Select all

cntr:= 2 List:=[] loop cntr { key:= val:= A_Index List[key]:= val ;List[A_Index]:= A_Index ;the same as above } msgBox("after Set") for key, val in List { msgBox(key " := " val) } for key, val in List { List.Delete(key) } msgBox("after Del") for key, val in List { msgBox(key " := " val) }
Delete method delete all but last one.
The Code represent how Delete method detele all but last item in the list (Associative Array).
Check when cntr:= 10 and cntr:= 100 the result List contain 1 item when all items Delete(byKey).
AHKv2.0 use the future now.
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

02 Jan 2018, 14:19

This is typical for-loop behaviour, from the docs,
Existing key-value pairs may be modified during the loop, but inserting or removing keys may cause some items to be skipped or enumerated multiple times
Same quote exists in v1 and v2.

Cheers.
User avatar
jeeswg
Posts: 5260
Joined: 19 Dec 2016, 01:58
Location: UK

Re: [BUG]Associative Arrays

02 Jan 2018, 15:02

- I discussed the issue of deleting keys in a loop in these links:
Easy Quesion about Array - AutoHotkey Community
https://autohotkey.com/boards/viewtopic ... 10#p170310
objects: loop and delete keys - AutoHotkey Community
https://autohotkey.com/boards/viewtopic.php?f=5&t=34202
- A backwards for loop might be useful cf. the registry loop, but I don't know what the programming costs of that would be.
- Note: listing the key names in a new array, and then deleting them in reverse order, may be the best approach, it's easier to remove a key from the end of an array than the beginning.

I also mentioned about multiple enumerators here, although these are more ideas, and not feature requests.
objects/object classes: new features from a newbie-friendly perspective - AutoHotkey Community
https://autohotkey.com/boards/viewtopic.php?f=5&t=42135

Code: Select all

q:: ;an unsuccessful attempt at deleting each key in a for loop (half the keys remain) oArray := {} Loop, 26 oArray[Chr(96+A_Index)] := Chr(96+A_Index) for vKey, vValue in oArray oArray.Delete(vKey) vOutput := "" for vKey, vValue in oArray vOutput .= vKey " " vValue "`r`n" ;oArray := "" MsgBox, % vOutput return ;================================================== w:: ;attempt to delete each key in a for loop oArray := {} Loop, 26 oArray[Chr(96+A_Index)] := Chr(96+A_Index) oRemove := [] for vKey in oArray oRemove.Push(vKey) vOutput := "" for vKey, vValue in oArray vOutput .= vKey " " vValue "`r`n" MsgBox, % vOutput Loop, % vIndex := oRemove.Length() oArray.Delete(oRemove[vIndex--]) oRemove := "" vOutput := "" for vKey, vValue in oArray vOutput .= vKey " " vValue "`r`n" ;oArray := "" MsgBox, % vOutput return
[EDIT:] Added failed and successful remove key attempts.
homepage | tutorials
[code boxes are currently not working 100%]
[click the 'Reply with quote' button on a post to see the full indented text]
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

03 Jan 2018, 06:49

https://lexikos.github.io/v2/docs/commands/For.htm
Existing key-value pairs may be modified during the loop, but inserting or removing keys may cause some items to be skipped or enumerated multiple times. One workaround is to build a list of keys to remove, then use a second loop to remove the keys after the first loop completes.
Removing ALL items by key (short solution) by only ONE loop

Code: Select all

list:=[] loop 13 { list[A_Index * 111]:= A_Index } ISEMPTY: ; the code -------------- for key in list { list.Delete(key) goto("ISEMPTY") } ; -------------------------- msgBox("Delete complete!") for key, val in List { msgBox(key " := " val) }
list contain 13 key:val pairs and the code loops exactly 13 times.
In other words loops key numbers times.
AHKv2.0 use the future now.
User avatar
nnnik
Posts: 3427
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: [BUG]Associative Arrays  Topic is solved

03 Jan 2018, 11:07

The easiest solution would be:

Code: Select all

for key, val in list.clone() { list.delete( key )
This way doubles the array in lists for the duration of the for loop - but unless the list contains a massive amount of data such as strings it shouldn't be an issue.
Recommends AHK Studio
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

03 Jan 2018, 13:06

Ofc, if you need to do this you have a design flaw in your program, please help my poor imagination :D . If you need to dispose of the items in a list, then list := [] is going to be fastest, and easiest, but not equivalent. If you only want do delete integer keys, you can easily use the delete range functionallity of delete, eg,

Code: Select all

list.delete(1, list.length()) ; alt. min/maxIndex
You can delete a range of string keys too.

Cheers.
User avatar
nnnik
Posts: 3427
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: [BUG]Associative Arrays

03 Jan 2018, 13:40

OK
Imagine you have a Game Loop iterating over any entity inside the game world. The happenings inside the game world, might end up deleting the entity, thus removing it from the list currently being iterated over.
It took me ~6 hours of debugging since the object that should have been deleted was not iterated over since I forgot about the for loop issue and AHK exited ungracefully - I was constantly looking for a circular reference or a stack overflow or an issue with the connected COM plugin only to find nothing.
There are tons of points where you iterate over elements in a list and take actions on them that might end up deleting them.
Recommends AHK Studio
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

03 Jan 2018, 14:00

The easiest solution would be:

Code: Select all

for key, val in list.clone() { list.delete( key )
This way doubles the array in lists for the duration of the for loop - but unless the list contains a massive amount of data such as strings it shouldn't be an issue.
Not acceptable for me due to duplication of pairs.

Code: Select all

class Task { static List:=[] __new(name) { Task.List[this]:= name ;key:reference to object | val:some string ;run(name) ;run no wait } del() { ;close ProcessXXX ;clear ProcessXXX aditional files ;and so on Task.List.Delete(this) ;free last reference to object } } onExit("__onExit") loop 10 new Task("Process" A_Index) #!F4::ExitApp __onExit { ISEMPTY: for key in Task.List { key.del() goto("ISEMPTY") } }
Array.Delete(key) must clear reference to object.
AHKv2.0 use the future now.
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

03 Jan 2018, 14:01

There are tons of points where you iterate over elements in a list and take actions on them that might end up deleting them.
Thank you nnnik, however, I have no problems imagining the need to delete items from a list, my imagination is lacking when it comes to the need of deleting all items unconditionally, where list := "" wouldn't do the job, or where you couldn't design your program such that it would.
It took me ~6 hours of debugging since the object that should have been deleted was not iterated over since I forgot about the for loop issue
I've been there too, unfortunately more than once, and I don't trust myself to not end up there again :( :lol:.

Cheers.
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

03 Jan 2018, 14:09

@ _3D_, once the loop is over, the cloned object is released. In your example, you could do task.list := [] ( :oops: Edit: and __delete instead of del() ) ;)
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

03 Jan 2018, 16:06

Reason to keep del() instead of __delete()

Code: Select all

class Task { static List:=[] __new(name, mode) { this.mode:= mode Task.List[this]:= name } del() { msgBox(Task.List[this] "|" this.mode) Task.List.Delete(this) ;call destructor (__delete) } __delete() { msgBox(Task.List[this] "|" this.mode) ;This.List[this] not exist ;Task.List.Delete(this) ;call destructor (__delete) } } new Task("task1", "222 222") ;task1 new Task("task3", "123 123") ;task3 new Task("task3", "456 456") ;task3 again new Task("task1", "111 111") ;task1 again CLOSE_TASK3: ;close first for key, val in Task.List { if inStr(val, "3") { ;conditional destruct key.del() goto("CLOSE_TASK3") } } CLOSE_ALL: ;close all for key, val in Task.List { key.del() goto("CLOSE_ALL") }
AHKv2.0 use the future now.
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

04 Jan 2018, 07:12

Reason to keep del() instead of __delete()
Indeed, and the reason is a flawed design ;).

Code: Select all

CLOSE_TASK3: ;close first for key, val in Task.List { if inStr(val, "3") { ;conditional destruct key.del() goto("CLOSE_TASK3") } }
That is probably the most inefficent way one could think of. I suggest, as per the [url=https://lexikos.github.io/v2/docs/commands/For.htm]for[/url] documentation,
build a list of keys to remove, then use a second loop to remove the keys after the first loop completes.
Cheers.
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

04 Jan 2018, 14:16

Reason to keep del() instead of __delete()
Indeed, and the reason is a flawed design ;).

Code: Select all

CLOSE_TASK3: ;close first for key, val in Task.List { if inStr(val, "3") { ;conditional destruct key.del() goto("CLOSE_TASK3") } }
That is probably the most inefficent way one could think of. I suggest, as per the [url=https://lexikos.github.io/v2/docs/commands/For.htm]for[/url] documentation,
build a list of keys to remove, then use a second loop to remove the keys after the first loop completes.
Cheers.
Let I guest if we have 100 pairs:
- first need 100 iterations to determinate "right" keys
- second need +number iterations to delete "right" keys
SO my code will do ALL with 100 iterations instead of 100 + number. (think about 100 + 55 it mean 55% more iterations)
:beer: Cheers (sorry no wine)
AHKv2.0 use the future now.
User avatar
nnnik
Posts: 3427
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: [BUG]Associative Arrays

04 Jan 2018, 21:18

Your code will never terminate if a single val contains the string "3".
But yeah what makes your code slow is the the constant construction and destruction of iterators.
Recommends AHK Studio
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

05 Jan 2018, 06:13

Without comment about slow code.
Anyone can try.

Code: Select all

class Task { static time:= 0 static tick:= 0 static List:= [] __new(name, mode:= 0x13) { this.mode:= mode this.name:= name Task.List[this]:= "" ;no value for key } end() { ;initate destruct if !Task.time Task.time:= A_TickCount Task.List.Delete(this) ;self kill } ;Task.List.Delete(this) ;Task.List:= "" __delete() { ;destruct idea by Helgef if !Task.time Task.time:= A_TickCount this.name:= "" Task.tick:= A_TickCount - Task.time } } result:= "" count:= 50000 ; SLOW ----------------------------------------------- loop count new Task("name" A_Index) Task.time:= "" SLOW: for key in Task.List { key.end() goto("SLOW") } msgBOX(result.= "SLOW`t:= " Task.tick "`n") ; FAST ----------------------------------------------- array:= [] loop count new Task("name" A_Index) Task.time:= "" for key in Task.List array[A_Index]:= key loop array.length() array[A_Index].end() array:= "" msgBOX(result.= "FAST`t:= " Task.tick "`n") ; ENUM ----------------------------------------------- loop count new Task("name" A_Index) Task.time:= "" while (_enum:= Task.List._newEnum()).Next(key, val) { loop { key.end() } until !_enum.Next(key, val) } msgBOX(result.= "ENUM`t:= " Task.tick "`n") ; LOOP ----------------------------------------------- loop count new Task("name" A_Index) Task.time:= "" loop { isEmpty:= 1 for key in Task.List { key.end() isEmpty:= 0 } } until isEmpty msgBOX(result.= "LOOP`t:= " Task.tick "`n") ; Helgef --------------------------------------------- loop count new Task("name" A_Index) Task.time:= "" Task.List:= [] ;Task.List:= "" change type from Array to string if further use msgBOX("Wait for Helgef",, "T2") ;this 2 seconds is out of tick (destruction in background) msgBOX(result.= "HELL`t:= " Task.tick "`n")
Conclusions:
1. SLOW is slowest (nnnik is right)
2. FAST is slower (double counting is any time slow).
3. LOOP and Task.List:= [] is EQUAL fastest.
While the array is being destroyed (Array:= []), all keys are enumerated correctly no mater that they are destroyed (deleted) at the same time.
AHKv2.0 use the future now.
User avatar
nnnik
Posts: 3427
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: [BUG]Associative Arrays

05 Jan 2018, 11:58

How does the .clone method compare to that?
Recommends AHK Studio
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

05 Jan 2018, 14:04

Hello _3D_ :wave:
Making the enumerators is a waste indeed, but for the particular code I commented on, that is, CLOSE_TASK3:, the main problem isn't the time it takes to make the enumerators, at least not in general. The key issue with CLOSE_TASK3:, which is implied by nnnik's comment, but which you seem to have failed to realise, is that for every key which meets the condition (instr(...)) you have to recheck the condition for all preceeding keys before finding the next match. Horror example, 10000 items, 5000 of them matches the condition, and they are all situated at the end of the list, the two-loop method makes two enumerators, 10000 checks of the condition and 5000 delete iterations, CLOSE_TASK3 makes 25 005 000 checks and 5001 enumerators. Here, the significant part isn't the enumerators, and the facepalm grows at a square rate.

In your test script, your redesign of the class is probably what I would have suggested. You are not using List to store the name, but you put it where it belongs, and you do the (symbolic) clean up in __delete and have the method del only to clear the circular reference :thumbup:. However, the benchmarking is flawed, I think I will use this for a new puzzle :think:.

Cheers :beer: .

Edit: Puzzle 9: Release the bug.
User avatar
jeeswg
Posts: 5260
Joined: 19 Dec 2016, 01:58
Location: UK

Re: [BUG]Associative Arrays

05 Jan 2018, 14:53

- Another approach. An example to in effect remove keys from an array, by 'renaming' it, creating a new array with the old name, and moving keys from the old array to the new array.
object reference count of 0 crashes script (ObjAddRef/ObjRelease) - AutoHotkey Community
https://autohotkey.com/boards/viewtopic ... 49#p192849
- If you're going to be searching the text of multiple array items, it might be better to use an LF-delimited string, or perhaps even an LF-delimited string *and* an array (maintained simultaneously), or use a 'StrJoin' function when necessary.
homepage | tutorials
[code boxes are currently not working 100%]
[click the 'Reply with quote' button on a post to see the full indented text]
_3D_
Posts: 188
Joined: 29 Jan 2014, 14:40

Re: [BUG]Associative Arrays

06 Jan 2018, 10:26

Hello _3D_ :wave:
I try to move all termination actions to __delete() but ... And in last I kept __del() - removing references and __delete() to other actions.

Code: Select all

class Task { static List:= [] __new(name) { this.name:= name this.mode:= 0 Task.List[this]:= "" } static __del:= func("__del") __delete() { if this.mode != "" { ;prevent second run msgBox(this.name " __delete()") this.mode:= this.name:= "" ;prevent second run } } } __del(byRef this) { ;clear reference in list and in calling var Task.List.Delete(this) this.__delete() ;forced __delete() if other references this:= "" ;autocall __delete() if no more references } var:= new Task("VAR") loop 5 new Task("LOOP " A_Index) for key in Task.List msgBox(key.name " in List") var.__del() ;var:= "" cant clear reference in list msgBox(type(var)) ; Task.List:= [] ;Helgef ------- for key in Task.List msgBox(key.name " in List")
It is not so quite stable and cleared but will work. class Task
Thanks for ideas.
Last edited by _3D_ on 07 Jan 2018, 09:54, edited 1 time in total.
AHKv2.0 use the future now.
Helgef
Posts: 3221
Joined: 17 Jul 2016, 01:02
Contact:

Re: [BUG]Associative Arrays

07 Jan 2018, 07:00

Hello.

Code: Select all

__del(byRef this) { ;clear reference in list and in calling var Task.List.Delete(this) ;this.__delete() ;forced __delete() if other references this:= "" ;autocall __delete() if no more references }
:) I like that, nifty! Only downside is you need the function. Slightly less convenient but no function and same result would be

Code: Select all

myTask := new Task Task.del(myTask) class task { ; ... del(byref r){ Task.List.Delete(r) r := "" } ; ... }
Thanks for the idea ;) Cheers :beer:

Return to “AutoHotkey v2 Development”

Who is online

Users browsing this forum: No registered users and 4 guests