MaxIndex() and MinIndex() method not converted properly.

[srourbrendan]

In v1, MaxIndex() is an object method, therefore supported by arrays. In v2, the Array class does not have a MaxIndex() or MinIndex() method, causing converted code to give errors.

v1 code:

F1::
x := [1, , 3]
MsgBox % x.MaxIndex()
Return

Generated v2 code:

F1::
{
x := [1, , 3]
MsgBox(x.MaxIndex())
Return
} 

Running the generated code as is will give the following error:

Error: This value of type "Array" has no method named "MaxIndex".

Note: MaxIndex() does exist in the v2 docs, as a method for the ComObjArray class.

In my case, I manually converted x.MaxIndex() to x.Length. While they are technically different conceptually, functionally they yield the same value. I'm not sure if that works as a full solution though.

[Banaanae]

Maybe something like this?

x := [1, , 3]
MsgBox x.Length != 0 ? x.Length : ""

x := []
MsgBox x.Length != 0 ? x.Length : ""

and for MinIndex

x := [1, , 3]
MsgBox x.Length != 0 ? 1 : ""

x := []
MsgBox x.Length != 0 ? 1 : ""

Afaik Max returns length and and Min return 1 unless array is blank? If so then this should work

[Banaanae]

^ MaxIndex() is fine
But MinIndex fails this example

MsgBox % {-10: 0, -1: 0}.MinIndex()  ;  -10

[mmikeww]

^what does MaxIndex return in that last example?

[Banaanae]

MsgBox % {-10: 0, -1: 0}.MaxIndex()  ;  -1
MsgBox % {-10: 0, -1: 0}.Length()  ;  0

These are more complicated than I originally thought, and may not have a perfect match in v2

[mmikeww]

yeah for now best to prob do nothing

[Banaanae]

Actually since map properties and array properties are converted separately, it's possible to use this solution just for arrays, then leave maps how they are for now

[andymbody]

Suggestion...

How about if the converter adds these custom properties (once) to the script when it encounters references to MinIndex() or MaxIndex()... And a note that communicates that Min/MaxIndex() should not be used for future scripts because it is not directly supported by v2. This will ensure that it is supported by the converted script, but also informs the user that this will not always be the case for new scripts.

Array.Prototype.DefineProp('MinIndex', {Call:(this)=>(i:='',e:=this.__Enum(),[((*)=>e(&k,&v)&&(i:=(i=''||k<i)?k:i))*],i)})
Array.Prototype.DefineProp('MaxIndex', {Call:(this)=>(i:='',e:=this.__Enum(),[((*)=>e(&k,&v)&&(i:=(i=''||i<k)?k:i))*],i)})

; Array example
a := []
MsgBox '[' a.minindex() ']'	; (empty string)
MsgBox '[' a.maxindex() ']'	; (empty string)
a := [100,200,300]
MsgBox '[' a.minindex() ']'	; [1]
MsgBox '[' a.maxindex() ']'	; [3]

Should work for maps as well, with an extra check

Map.Prototype.DefineProp('MinIndex',{Call:(this)=>(i:='',e:=this.__Enum(),[((*)=>e(&k,&v)&&(i:=(isNumber(k)&&(i=''||k<i))?k:i))*],i)})
Map.Prototype.DefineProp('MaxIndex',{Call:(this)=>(i:='',e:=this.__Enum(),[((*)=>e(&k,&v)&&(i:=(isNumber(k)&&(i=''||i<k))?k:i))*],i)})

; Map example
m := Map()
MsgBox '[' m.minindex() ']'	; (empty string)
MsgBox '[' m.maxindex() ']'	; (empty string)
m := Map(-10,0, -1,0, -5,0, -11,0, "key5","val5")
MsgBox '[' m.minindex() ']'	; [-11]
MsgBox '[' m.maxindex() ']'	; [-1]

Although, AFAIK, this object literal is not valid in v2
obj := {-10: 0, -1: 0}

[Banaanae]

This would work, but it feels like it goes against the purpose of this script. Like in #100 the main purpose of the script is to ease the transition to into V2.

However we could link this issue, so if people do need MinIndex and MaxIndex they could add it themselves?

@mmikeww do you have any ideas?

[andymbody]

This would work, but it feels like it goes against the purpose of this script. Like in #100 the main purpose of the script is to ease the transition to into V2.

However we could link this issue, so if people do need MinIndex and MaxIndex they could add it themselves?

Yeah... it is stepping slightly out of bounds, but there is no direct conversion for Min/MaxIndex() between v1 and v2. This would allow the converted script to support these unsupported methods. Which helps ease the burden of manual edit for the user.

I did add this solution to the script section of the forum in case anyone wants to add it manually.

[mmikeww]

my initial reaction is that that is kinda out of scope for the converter. but i dont really know how it should handle it

[andymbody]

I read the comments for #100. I see that once an exception is made, it may lead to an expectation for other situations to be supported as well. I wonder if a secondary tool might be better to address some of these "exceptions" that can be run after the conversion with this tool? Just thinking out loud...

[Banaanae]

I read the comments for #100. I see that once an exception is made, it may lead to an expectation for other situations to be supported as well. I wonder if a secondary tool might be better to address some of these "exceptions" that can be run after the conversion with this tool? Just thinking out loud...

It could be possible to add these inside the Quick convertor menu, we can make a PR to add these changes, as there's atleast two other issues that could benefit from this

[Banaanae]

Made a quick script for this and found MinIndex fails with this example MsgBox % [ , , 1, 2, 3].MinIndex() ; 3 but above fix returns 1

[andymbody]

Made a quick script for this and found MinIndex fails with this example MsgBox % [ , , 1, 2, 3].MinIndex() ; 3 but above fix returns 1

I misunderstood 'Returns the lowest or highest integer key, if present.'

See below for the corrected versions...

[andymbody]

ok... here is the corrected custom properties that should return the same value as v1

Array.Prototype.DefineProp('MinIndex', {Call:(a)=>(i:='',e:=a.__Enum(),[((*)=>e(&k,&v)&&(i:=k,!IsSet(v)))*],i)})
Array.Prototype.DefineProp('MaxIndex', {Call:(a)=>(i:='',e:=a.__Enum(),[((*)=>e(&k,&v)&&(i:=IsSet(k)?k:i))*],i)})
; examples
MsgBox [,"two","three",,"five",].MinIndex()	;2
MsgBox [,"two","three",,"five",].MaxIndex()	;5
MsgBox [ , , 1, 2, 3].MinIndex()		;3
MsgBox [ , , 1, 2, 3].MaxIndex()		;5
MsgBox [].MinIndex()				; (empty string)
MsgBox [].MaxIndex()				; (empty string)

I think a map is stored with keys in alphabetical order, so not sure how helpful Min/MaxIndex will be for maps. But it does work, if you decide to include Min/MaxIndex for maps.

[Banaanae]

Update on this issue:
Object methods and array methods can be converted separately
We can use #178 (comment) for arrays (MinIndex will require a warning as it isn't a 100% fix)
Objects are unlikely to receive a fix (just too different from v1)
The fix for arrays is challenging as with the current setup we can only edit MaxIndex() in MyArray.MaxIndex() and can't access the required MyArray bit (as my solution requires it twice), still possible to fix but we need to do it post convert