GuiCtrl.GetPos()

Discuss the future of the AutoHotkey language
User avatar
kczx3
Posts: 1116
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: 6960
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: 1116
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: 1116
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: 6960
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: 1116
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: 1116
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: 4440
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: 1116
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.
lexikos
Posts: 6960
Joined: 30 Sep 2013, 04:07
GitHub: Lexikos

Re: GuiCtrl.GetPos()

05 Jul 2020, 22:42

kczx3 wrote:
01 Jul 2020, 19:48
Uh, that was your suggestion.
No, my suggestion was to add properties for cases like your example, where you only retrieve one coordinate from each control at a time. It was not "to atomically get them all via properties". If you need to atomically get them all, you can do that with GetPos.
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.
That was part of the intention.
Will the GuiCtrl class be exposed directly like Gui in the future?
Yes. Probably as properties of Gui (I think the current type names imply that).
Helgef wrote:
02 Jul 2020, 03:19
I can easily see myself doing a mistake like getpos ,w.
Did you mean GetPos().w or omitting the two commas needed to make w width? I don't see why you'd make the latter mistake.

I still don't really understand the preference for Pos returning an object. I never would have designed it that way; it was that way only because fincs chose it.


I can imagine a version of the AutoHotkey library (perhaps exported to JavaScript or another language without ByRef) where objects are returned, but consistently across all functions that currently use ByRef. I just choose not to make AutoHotkey v2.0 that way.
Helgef
Posts: 4440
Joined: 17 Jul 2016, 01:02
Contact:

Re: GuiCtrl.GetPos()

06 Jul 2020, 03:07

I don't see why you'd make the latter mistake.
I meant the latter. I can make mistakes for several reasons, for example,
  • Sloppiness.
  • Lack of concentration.
  • Lack of talent.
  • Poor motor skills.
I've made similar mistakes many times, i.e., missing commas. Most often it means I pass the wrong value for the parameter rather than retrieving the wrong output value. Passing the wrong value can be immediately obvious, but getting the wrong output is more difficult to debug.
I still don't really understand the preference for Pos returning an object.
Imo, out := f() is clearer than f(out). The object gives some uniformity to the code, since the property names are the same regardless of what you name the variable which holds the object.

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

Re: GuiCtrl.GetPos()

06 Jul 2020, 04:53

I thought that the general consensus in the AHK community regarding ByRef parameters is that they shouldn't be used.
All efforts in v2 aim to remove commands - one of the main users of ByRef parameters.
ByRef values first need to be transformed to put them into objects. You need to do all kinds of things to properly return them.
ByRef values that are used across multiple functions will often lead to globals or just very long spaghetti code functions.

getPos follows a getter convention naming wise. Yet acts totally different due to its nature of retrieving them ByRef.
To make it intuitive that getPos is read-only we completely make getPos unintuitive to use.
Intuitively one would use getPos like: return := gui.getPos() but that will yield nothing.

Generally speaking I find that there is a large division amongst the community into 2 major factions.
There is the faction that generally uses a lot of globals, functions and ByRef parameters and avoid objects.
And there is the faction that generally uses objects and avoids byRefs and globals.
GuiControl.getPos(x, y, w, h) is exactly made so that it doesn't satisfy anyone. If you go with object style I and probably everyone else expects it to return an object.

Lastly I want to talk about something more general.
AutoHotkey is a language that's very concerned with many positions on the screen. Whether its your mouse, images that were found or a GUI the script creates.
AutoHotkey cares about positions and rectangles on the screen. Its one of its core components.
How is it possible that in such a language there is not a single data structure or function dedicated to calculating with those positions?
Is having 4 seperate variables really the peak evolution of structuring RECT data? Is that really what you think?

Anyways enough rambling from my part.
How is your day going?
Recommends AHK Studio
User avatar
kczx3
Posts: 1116
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

06 Jul 2020, 09:36

I echo some of @nnnik's comments, though I can't speak to "the general consensus in the AHK community regarding ByRef parameters". I am not fond of them personally.

Doesn't the AHK source retrieve all four of XYWH even if the users only passes one ByRef parameter to GetPos()? If you wanted to limit what is returned in the object from GetPos then maybe have it accept a single string that can contain one or more of X, Y, W, H. The returned object would only them contain those properties. I have no problem always getting all 4 though.
User avatar
nnnik
Posts: 4466
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: GuiCtrl.GetPos()

06 Jul 2020, 16:22

I kind of wrote this part a bit before the others and I kind of contradict myself later on because I say that there is a group of people that does not agree. I basically fail to highlight that it is the opinion of the major part of the community. I was just done with the post at that point should have just scrapped the statement.
Recommends AHK Studio
lexikos
Posts: 6960
Joined: 30 Sep 2013, 04:07
GitHub: Lexikos

Re: GuiCtrl.GetPos()

06 Jul 2020, 23:47

@kczx3
I was originally alluding to general wastefulness. I am not concerned about performance, and have merely been debating topics you have brought up. I've already suggested that gc.GetPos() could be extended to return an object; if there's any reason this hasn't happened yet, it is unrelated to performance.

Are you suggesting something like gc.GetPos("xy")? As with your suggestion for a parameter to control caching, you'd have to convince me to remove the ByRef parameters first. But your suggestions only address performance, which I care even less about after benchmarking than I did before, if that's possible.

Who do you think would actually use the parameter, if the default gives all properties?

I changed gc.Pos to gc.GetPos(x,y,w,h) fundamentally because I felt it was better - everything else was secondary (perhaps rationalization).


@nnnik
I think you see what you want to see, like most people. Anything one individual says about the opinions of other people, let alone the majority, I am not likely to take seriously unless I've seen it for myself.
Helgef
Posts: 4440
Joined: 17 Jul 2016, 01:02
Contact:

Re: GuiCtrl.GetPos()

07 Jul 2020, 03:09

I've already suggested that gc.GetPos() could be extended to return an object
I think it is better to let the users do this for themselves. We have been given good tools to modify things to our liking, no need to build in multiple behaviours for these functions, imo. Although I would prefer pos:=getpos(), move(pos) only. Or setPos rather than move, I guess.

I think there are more important fish to fry than this one though, cheers :fish:
User avatar
nnnik
Posts: 4466
Joined: 30 Sep 2013, 01:01
Location: Germany

Re: GuiCtrl.GetPos()

07 Jul 2020, 03:39

Well then why don't you let people vote on this one?
Recommends AHK Studio
User avatar
kczx3
Posts: 1116
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

07 Jul 2020, 08:34

@lexikos I don't know that continuing this conversation is helping. Its obvious that we have differing ways of discussion and the levels at which you and I comprehend concepts are vastly different. I really don't care of negligible performance. The only reason I brought up performancr was due to some of the reasons you listed in your change notes and I was trying to think of ways to compromise between how you wanted it and how I would like it.
lexikos wrote:Are you suggesting something like gc.GetPos("xy")?
Yeah that's what I was thinking simply as a way to keep the ability for the user to only retrieve the data they want as can currently be done with your ByRef implementation. Again, it was just a means to achieve a compromise. I was perfectly fine with how this all worked before GetPos. I'm not sure who would use such a parameter. I would not.
lexikos wrote:...because I felt it was better
Basically, my initial post was simply to say "I disagree". I was trying to critically think to the best of my ability to come up with logical reasons as to why GetPos was worse than what it replaced. It seems my efforts have failed and all I have is my opinion based on how I used the prior functionality.
User avatar
vvhitevvizard
Posts: 400
Joined: 25 Nov 2018, 10:15
Location: Russia

Re: GuiCtrl.GetPos()

16 Jul 2020, 07:30

@kczx3, on the contrary, in https://www.autohotkey.com/boards/viewtopic.php?f=37&t=77841
I suggested to expand that new gc.getpos(var,var,var,var) style with:
gc.setpos(), gc.settextstyle( [bold/italic/other flags,size, kerning/etc]), gc.setcolor(color [,background]), gc.autosize, gc.settextalign(), gc.padding, gc.marginx, gc.marginy, etc. All of them using vars-arrays-byrefvars instead of "parameters string". I find single opt.("parameters string") approach to be not so convenient for changing gui control's options programmatically (anchoring, dynamically changing proportions, color/background color/font styles, etc) and also it differs greatly from other languages' approach, it makes it harder to port something to-from other languages.
User avatar
kczx3
Posts: 1116
Joined: 06 Oct 2015, 21:39

Re: GuiCtrl.GetPos()

16 Jul 2020, 08:14

@vvhitevvizard the whole “single opt parameter string” was meant to bridge the gap. My preference is still either a method or property that returns an object as it used to.

Return to “AutoHotkey v2 Development”

Who is online

Users browsing this forum: No registered users and 7 guests