GuiCtrl.GetPos()

Discuss the future of the AutoHotkey language
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

GuiCtrl.GetPos()

30 Jun 2020, 20:18

Looking through my GUI code, I'm finding that the changes from GuiCtrl.Pos to GuiCtrl.GetPos() is a huge dissatisfier for my coding style. At the end is a sample of a program I have been working on. I use the coordinates of prior controls A LOT for positioning subsequent controls just right. By moving all these into ByRef parameters it makes the code much uglier IMO and harder to use. One of your arguments for this change is:
No need to remember W vs. Width, H vs. Height, just the typical parameter order.
Honestly, I find the "typical parameter order" harder to remember than simple object properties. The letters are easier to remember than the parameter order (again, in my opinion). Additionally, I usually don't need all of them. And while it may be more efficient to only retrieve those that you'll use, having to remember which parameter to pass a var name to and which to skip is annoying and causes me to look at the docs much more frequently. It also creates a lot of spaghetti code with missing parameters. Code becomes bloated as well because now every byref parameter I pass in for each unique control must have a unique name. So instead of scoping the position values with a single unified object, I must prefix all of the properties variables with that same similar scoping prefix, ex: myCtrl.GetPos(myCtrlX, myCtrlY, myCtrlH, myCtrlW).

Another argument was:
Avoids the gc.Pos.X, gc.Pos.Y, etc. anti-pattern (repeatedly retrieving window position and creating multiple objects).
If that's a valid concern, then I propose the following. Change GuiCtrl.GetPos to return the same object that GuiCtrl.Pos used to but cache the coordinates. Instead, have the method accept a single, optional parameter that forces the window position to be re-queried and a new object created (and re-cached).

Would love to hear other's thoughts on this topic.

Code: Select all

main := Gui.new("+Resize +MinSize", "User Search")
main.OnEvent("size", "mainResize")
main.OnEvent("close", (*) => main.hide())

; off screen default button for searching via Enter
main.addButton("x-100 w25 Default", "Submit").OnEvent("click", "userSearch")

searchBox := main.addEdit("xm ym w150 Section vSearchBox 0x2000000")
SetEditCueBanner(searchBox.hwnd, "Search")
EM_SETMARGIN(searchBox.hwnd, 0, 20)
resetSearchFn := Func("resetSearch").bind(searchBox)

searchClearGui := Gui.new("-Caption +Parent" searchBox.hwnd " 0x40000000")
searchClearGui.MarginX := searchClearGui.MarginY := 0
searchClearGui.BackColor := "White"
searchClear := searchClearGui.addButton("w15 h15", Chr(10005))
searchClear.OnEvent("Click", resetSearchFn)
searchClearGui.Show("x" searchBox.pos.w - 20)

main.addRadio("xs+20 vsearchType Checked", "Users").OnEvent("Click", resetSearchFn)
main.addRadio("yp", "Groups").OnEvent("Click", resetSearchFn)

resultsLV := main.addListView("r10 xs w150 vresults -Multi +LV0x4000", "Users")
resultsLV.OnEvent("ItemSelect", "handleSelection")
resultsLV.OnNotify(LVN_GETEMPTYMARKUP := -187, "lvMarkupHandler")
SetExplorerTheme(resultsLV.hwnd)

grouper := main.addGroupBox("ys-6 w300 h" searchBox.pos.h + resultsLV.pos.h + main["searchType"].pos.h + (main.MarginY * 3) " vGrouper")

main.setFont("s14")
main.addEdit("xp+5 yp+10 wp-10 r2 Center vName ReadOnly -E0x200 0x1 -VScroll")
; main.addEdit("y+-" main.MarginY " wp Center vTitle ReadOnly -E0x200")
main.setFont()

detailsLV := main.addListView("xp-2 y+m wp r" detailFields.length " vDetails Hidden BackgroundEEEEEE -E0x200 -Hdr +LV0x4000", "Label|Value")
detailsLV.OnEvent("ContextMenu", "lvContext")
for i, field in detailFields {
    detailsLV.Add("", field)
}
detailsLV.ModifyCol(1)
width := SendMessage(LVM_GETCOLUMNWIDTH := 0x101D, 0, 0, detailsLV.hwnd)
detailsLV.ModifyCol(2, detailsLV.pos.w - width)

membershipLV := main.addListView("ys h" grouper.pos.h - 6 " w250 vGroups +LV0x4000 Sort", "AD Groups")
membershipLV.OnEvent("ContextMenu", "lvContext")
SetExplorerTheme(membershipLV.hwnd)
membershipLV.ModifyCol(1, membershipLV.pos.w - SysGet(2) - (SysGet(45) * 2))
lexikos
Posts: 6854
Joined: 30 Sep 2013, 04:07
GitHub: Lexikos

Re: GuiCtrl.GetPos()

01 Jul 2020, 06:01

You haven't been using WinMove, WinGetPos, ControlGetPos, ControlMove, or other functions where two sets of coordinates are used in sequence? The order X, Y, Width, Height is used across many languages and frameworks.

The other reasons I gave were:
  • Avoids the gc.Pos.X += 10 trap (this had no effect).
  • More consistent with WinGetPos, WinGetClientPos and ControlGetPos.
My main aim (for that moment) was consistency between all of the GetPos and Move functions/methods, but I had it in mind that gc.GetPos() could be extended later to return an object (when no parameters were passed), and Move could be extended to match. Compared to gc.Pos.X, I think it's much clearer that gc.GetPos().X += 10 is not operating on gc's pos (and not going to work).
Instead, have the method accept a single, optional parameter that forces the window position to be re-queried and a new object created (and re-cached).
Then the method would be inconsistent with everything else, again, and would no longer support retrieving the positions directly into variables. [Edit: Or maybe you meant an additional parameter?] I generally find there's enough meaning in a one or two-letter variable name, within a limited context/scope, and don't want the extra noise of the variable containing the object and .. However, it might be that I'm working with bounding rectangles more often than single coordinates, and I can see your point about the ByRef method bloating your code.

If the presence of the optional parameter forces the window position to be re-queried, that implies it wouldn't be by default, which may cause headaches. Perhaps you meant that the default would be to get the position every time, with the parameter available for someone with performance in mind. But for performance, it would be more effective to use the current method, or even retrieve the object once and put it in a variable. It is very unlikely that the cost of retrieving the position, allocating the object and four strings, and initializing four name-value pairs, would be high enough to justify the added complexity and "polluting" the interface and documentation; especially in the context of a GUI script.

For testing, I modified GetPos() to return an object (using almost the same code as the original Pos) when there are no parameters. According to a simple benchmark, calling it without parameters once takes 50% longer than calling ControlGetPos four times (passing the GuiControl object), and 500% longer than calling GetPos() with parameters (in which case no object is created).

[Edit: Perhaps separate properties would be faster, more convenient and less problematic? For a Gui object, they might not match if queried in order while the window is being moved, but that's not an issue if you're only using one at a time.]
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

01 Jul 2020, 06:32

You have deduced correctly that its quite infrequent that I am operating on windows or controls not owned by my AHK scripts. When it came to moving a gui or control that I created, I didn't have to worry about parameter order at least due to the options parsing. Just each of the prefixes (X, Y, W, and H). And it never confused me between W or Width.

I hadn't come across trying to do gc.Pos.X += 10 so it never crossed my mind. To me, returning the object just seems more useful or user-friendly. I'd prefer any function that returns position and size to return an object. Consistency, or lack of, never bothered me in these scenarios although I do appreciate uniformity.

You interpreted incorrectly. I meant changing GetPos() to accept only a single parameter to flex whether cached position and size are returned or not. If you wanted consistency, then it would be my recommendation to make all functions that retrieve position work is this same manner. Maybe you're right that the window position should always be re-queried by default. The parameter can be used for performance optimization.

In the grand scheme of things, I'm not worried performance (especially knowing the call to return an object takes 1µs).

Obviously, this is definitely an opinion argument. You certainly have more facts to back up the changes you made. I just feel they are harder to work with as a programmer.
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

01 Jul 2020, 06:34

lexikos wrote:
01 Jul 2020, 06:01
[Edit: Perhaps separate properties would be faster, more convenient and less problematic? For a Gui object, they might not match if queried in order while the window is being moved, but that's not an issue if you're only using one at a time.]
This seems interesting. Is there no way to atomically get them all via properties to avoid pitfalls of retrieving during window movement?
lexikos
Posts: 6854
Joined: 30 Sep 2013, 04:07
GitHub: Lexikos

Re: GuiCtrl.GetPos()

01 Jul 2020, 17:17

Why via properties? I see the benefit when you're only retrieving one coordinate from each control at a time, but not so for multiple coordinates.

Where you may prefer to specify the prefix once when retrieving the values and every time thereafter (with .), I prefer to avoid specifying a redundant prefix at all (although sometimes I use a very short prefix), and especially a very short prefix containing .. Scope is only an issue if you are using the variables across more than a few lines.

An experiment:

Code: Select all

#Persistent ; Required due to a bug.
class ExtendsGuiAndControl {
    static __new() {
        g := Gui.New(), gcp := g.AddText().base.base, g.Destroy()
        for p in [Gui.Prototype, gcp]
            for n in this.Prototype.OwnProps()
                p.DefineProp(n, this.Prototype.GetOwnPropDesc(n))
    }
    X {
        get => (this.GetPos(x), x)
        set =>  this.Move(value)
    }
    Y {
        get => (this.GetPos(, y), y)
        set =>  this.Move(, value)
    }
    Width {
        get => (this.GetPos(,, w), w)
        set =>  this.Move(,, value)
    }
    Height {
        get => (this.GetPos(,,, h), h)
        set =>  this.Move(,,, value)
    }
}

; A simple test.
g := Gui.new()
g.AddButton(, "``n").OnEvent("Click"
    , (gc, *) => (
        gc.Y += gc.Height,
        gc.Gui.Height += gc.Height
    ))
g.AddButton("x+0", ">>").OnEvent("Click"
    , (gc, *) => (
        gc.X += gc.Width,
        gc.Gui.Width += gc.Width
    ))
g.Show()

If you wanted consistency, then it would be my recommendation to make all functions that retrieve position work is this same manner.
Caching for the last identified window/control? What if you're working with two windows? That seems even more problematic. I think that rather than telling the function to cache the values or object for you, it would be more efficient and more readable to do it yourself, with ordinary variables.
Consistency, or lack of, never bothered me in these scenarios although I do appreciate uniformity.
What's the difference between uniformity and consistency in this context?
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

01 Jul 2020, 19:48

lexikos wrote:
01 Jul 2020, 17:17
Why via properties?
Uh, that was your suggestion.

My point is in regards to using GetPos() when you have to do it on many controls on a gui. With the ByRef params, I have to prefix each needed variable with something that uniquely identifies it among the other controls position properties.

Forget the caching. It was simply a suggestion to improve performance since performance was one reason you changed how this works.
lexikos wrote:What's the difference between uniformity and consistency in this context?
I suppose I meant that I do appreciate uniformity across an API. I tend to be OCD about how my code looks and reads. I appreciate what you were trying to do to unify the methods for retrieving positions of windows/controls. I just prefer that the method return an object as the Pos property used to. I think the method variation would potentially steer people to call it once and store GetPos() in a variable and then use that to reference the X/Y/W/H, rather than retrieve it multiple times through the .Pos property.

No one else has responded yet so I'm inclined to think that the change doesn't bother anyone but me. I'm sure I'll get used to it. Just a bit of an inconvenience until it is learned/remembered.
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

01 Jul 2020, 19:54

Also, I forgot to mention that I quite enjoy your "experiment"! Will the GuiCtrl class be exposed directly like Gui in the future? Or would you have to create a control and retrieve gc.base.base as you did?
Helgef
Posts: 4397
Joined: 17 Jul 2016, 01:02
Contact:

Re: GuiCtrl.GetPos()

02 Jul 2020, 03:19

I like that getpos() and wingetpos() etc are consistent. But I always guessed (since getpos()) returning an object would be the norm. I would never return four values byref with my own udf for something which isn't performance critical, such as for a math function. I can easily see myself doing a mistake like getpos ,w.

It doesn't bother me very much though.

Cheers.
User avatar
kczx3
Posts: 1097
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

02 Jul 2020, 08:02

Helgef wrote:
02 Jul 2020, 03:19
I like that getpos() and wingetpos() etc are consistent. But I always guessed (since getpos()) returning an object would be the norm. I would never return four values byref with my own udf for something which isn't performance critical, such as for a math function. I can easily see myself doing a mistake like getpos ,w.
I think this echoes my thoughts well.

Return to “AutoHotkey v2 Development”

Who is online

Users browsing this forum: No registered users and 21 guests