To me, the best programming practice is using "byRef", because you know the function needs two parameters that are modified inside the function, and when you are calling the function you even know those parameters names.
If a function returns an array or associated array, the name of the function should be descriptive enough, so you don't need to read the "function help" to understand what is returning. In this case I prefer an array, because associated arrays needs parameters names and you could make a typo, something very difficult to discover when you debug.
Also try to do your owns functions following AHK commands, I mean, The same way AutoHotkey does, that way is more intuitive to use.
If I open an AHK file and I see "global" my stomach get up side down, you don't really know where the global variables are used and changed. This is a nightmare to follow the code, modify the code and debug. You can do it if the script is very simple, but in my opinion, any script get complex at the end, when you are constantly modifying it.
Code: Select all
FindTermOnWebAtXY(SearchTerm, ClickOnIt:=1, ByRef X:=0, ByRef Y:=0)
{
; Global ; TEST 19-10-2022
WinActivate, ahk_exe firefox.exe
WinWaitActive, ahk_exe firefox.exe
Sleep, 1000
Send, ^f
Sleep, 500
Send, % SearchTerm
Sleep, 2000
PixelSearch, X, Y, 0, 0, A_ScreenWidth, A_ScreenHeight, 0x78d838,, Fast
if ErrorLevel = 0
{
Found := 1 ; Return true if I found the pixels search
if (ClickOnIt ) ; more readable
MouseClick, % "Left", X, Y, 1, 10
else
Sleep, 100
}
else
{
; MsgBox, % SearchTerm . " niet gevonden!" ; not a good idea to hardcode a Msgbox inside a function, do it outside of the function
Found := 0 ; Return False if i didn't find
}
Return Found
}
if FindTermOnWebAtXY("Status", "Not", SearchX, SearchY)
{
SearchX := SearchX+2 ;<<<<<<<<<<<<<<<<<<<<<<
SearchY := SearchY+82
}
else {
; didn't find pixelSearch
MsgBox, % SearchTerm . " niet gevonden!" ; the user of the function do a Msgbox if he wants!
}
The name of the function is very important too:
- "SearchWeb" tells you there are infinite results as expected if you do a google search.
- "FindTermOnWebAtXY" tells you that you can find the results or not, a boolean value you can use.
This is called legibility of the code, it's easy human readable " if FindTermOnWebAtXY" than "if SearchWeb".
I always think the name of the function once I have coded it, because I don't know what exactly a function should do before writing the function. Once coded, I change the name of the function to something more descriptible
Edit: I miss the "clickOrNot" parameter name. You should always use positive values and verbs... I mean, instead of ClickOrNot:= "string here With UPcase and Lowercase" try to call it ClickOnIt := 1 (True, it must be clicked) or ClickOnIt := 0 (False, should NOT be clicked)