Code Optimization? - (Moved)

[keeeniiic]

Hello all,

my goal is to observe a open application and read the visual text, filter for a specific value and based on that value to refresh a sidebar using an http call.

This code is working but it is my first AutoIt Coding Experience and this is why I am asking you for any potential code optimization, as I have to observe many more Windows and in order to make this clean I ask for your assistance.

So the goals are:

Run invisible in the Background
Observe open Window of Application (only do something if the Windows is open)
Read Visual Text and get specific value (Vendor Number)
If this Number changes, update the sidebar using http call

Here is the code:

expandcollapse popup

#include "Array.au3"
#include "WinHttp.au3"
#include "WinAPI.au3"
#include "WindowsConstants.au3"
#include "StringConstants.au3"


Global $lieferant, $firstRun, $lieferantChanged
$firstRun = True

Func observe()
   While not WinExists("Test2 Rechnungskontrolle (Verbuchung)")
       Sleep(250)
   WEnd
   While WinExists("Test2 Rechnungskontrolle (Verbuchung)")
     Sleep(250)
     if($firstRun) Then
        getLieferant()
        $firstRun = False
      EndIf
     isChanged()
     getLieferant()
   Wend

EndFunc

Func getLieferant()

   if(WinExists("Test2 Rechnungskontrolle (Verbuchung)")) Then
      local $strString = WinGetText("Test2 Rechnungskontrolle (Verbuchung)")
      local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
      If IsArray($arrResult) Then $lieferant = $arrResult[0]
      Return $lieferant
   Else
      observe()
   EndIf

EndFunc

Func isChanged()

   if(WinExists("Test2 Rechnungskontrolle (Verbuchung)")) Then
      local $strString = WinGetText("Test2 Rechnungskontrolle (Verbuchung)")
      local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
      If IsArray($arrResult) Then $lieferantChanged = $arrResult[0]
      If $lieferantChanged == $lieferant Then
         getLieferant()
      Else
         ;MsgBox(0,"Changed",$lieferantChanged)
         ConsoleWrite($lieferantChanged)
         refreshSidebar($lieferantChanged)
         getLieferant()
      EndIf
   Else
      observe()
   EndIf
EndFunc

Func refreshSidebar($vendorID)
   Opt("MustDeclareVars", 1)

   ; Initialize and get session handle
   Global $hOpen = _WinHttpOpen()
   ; Get connection handle
   Global $hConnect = _WinHttpConnect($hOpen, "http://MKSRVAPP04:25024")

   Local $hRequest = _WinHttpSimpleRequest($hConnect, "GET", "api/client/sidebar/sendCommand?user=MULTIKRAFT\" & @UserName & "&call=OBJ:Vendor@@SELECT@@IDX:Vendor%20ID=" & $vendorID)

   ; Close handles
   _WinHttpCloseHandle($hConnect)
   _WinHttpCloseHandle($hOpen)
EndFunc



observe()

 

Thank you very much!

Best Regards
Kenan

[Melba23]

Moved to the appropriate forum, as the Developer General Discussion forum very clearly states:

Quote

General development and scripting discussions. If it's super geeky and you don't know where to put it - it's probably here.

---

Do not create AutoIt-related topics here, use the AutoIt General Help and Support or AutoIt Technical Discussion forums.

Moderation Team

[Nine]

Personally I use indirect recursion with much caution.  Also there is too much redundancy in your code.  Here what I would advise :

expandcollapse popup

Opt("MustDeclareVars", 1)

Global $lieferant = 0 ; make it to a value that cannot be found

observe()

Func observe()
  While True
    While Not WinExists("Test2 Rechnungskontrolle (Verbuchung)")
      Sleep(250)
    WEnd
    While WinExists("Test2 Rechnungskontrolle (Verbuchung)")
      Sleep(250)
      getLieferant()
    WEnd
  WEnd
EndFunc   ;==>observe

Func getLieferant()

  If Not WinExists("Test2 Rechnungskontrolle (Verbuchung)")) Then Return
  Local $strString = WinGetText("Test2 Rechnungskontrolle (Verbuchung)")
  Local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
  If Not IsArray($arrResult) Then Return
  If $lieferant == $arrResult[0] Then Return
  $lieferant = $arrResult[0]
  ;MsgBox(0,"Changed",$lieferant)
  ConsoleWrite($lieferant)
  refreshSidebar($lieferant)

EndFunc   ;==>getLieferant

Func refreshSidebar($vendorID)
  ; Initialize and get session handle
  Local $hOpen = _WinHttpOpen()
  ; Get connection handle
  Local $hConnect = _WinHttpConnect($hOpen, "http://MKSRVAPP04:25024")

  Local $hRequest = _WinHttpSimpleRequest($hConnect, "GET", "api/client/sidebar/sendCommand?user=MULTIKRAFT\" & @UserName & "&call=OBJ:Vendor@@SELECT@@IDX:Vendor%20ID=" & $vendorID)

  ; Close handles
  _WinHttpCloseHandle($hConnect)
  _WinHttpCloseHandle($hOpen)
EndFunc   ;==>refreshSidebar

Untested of course...

[keeeniiic]

56 minutes ago, Nine said:

Personally I use indirect recursion with much caution.  Also there is too much redundancy in your code.  Here what I would advise :

expandcollapse popup

Opt("MustDeclareVars", 1)

Global $lieferant = 0 ; make it to a value that cannot be found

observe()

Func observe()
  While True
    While Not WinExists("Test2 Rechnungskontrolle (Verbuchung)")
      Sleep(250)
    WEnd
    While WinExists("Test2 Rechnungskontrolle (Verbuchung)")
      Sleep(250)
      getLieferant()
    WEnd
  WEnd
EndFunc   ;==>observe

Func getLieferant()

  If Not WinExists("Test2 Rechnungskontrolle (Verbuchung)")) Then Return
  Local $strString = WinGetText("Test2 Rechnungskontrolle (Verbuchung)")
  Local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
  If Not IsArray($arrResult) Then Return
  If $lieferant == $arrResult[0] Then Return
  $lieferant = $arrResult[0]
  ;MsgBox(0,"Changed",$lieferant)
  ConsoleWrite($lieferant)
  refreshSidebar($lieferant)

EndFunc   ;==>getLieferant

Func refreshSidebar($vendorID)
  ; Initialize and get session handle
  Local $hOpen = _WinHttpOpen()
  ; Get connection handle
  Local $hConnect = _WinHttpConnect($hOpen, "http://MKSRVAPP04:25024")

  Local $hRequest = _WinHttpSimpleRequest($hConnect, "GET", "api/client/sidebar/sendCommand?user=MULTIKRAFT\" & @UserName & "&call=OBJ:Vendor@@SELECT@@IDX:Vendor%20ID=" & $vendorID)

  ; Close handles
  _WinHttpCloseHandle($hConnect)
  _WinHttpCloseHandle($hOpen)
EndFunc   ;==>refreshSidebar

Untested of course...

wow, this is great, thank you very much.

[Bilgus]

Next thing is you said you want to monitor multiple windows?

searching by window name is going to slow your code quite a bit

maybe something like this

expandcollapse popup

Global $aWindows[0][3] ;Hwnd, WinText, last vendorID

AddWindow($aWindows, "Test2 Rechnungskontrolle (Verbuchung)")
observe($aWindows)

Func AddWindow(Byref $aWin, $sWindowText, $VendorId = 0)
    Local $iElement = UBound($aWin, 1)
    ReDim $aWin[$iElement + 1][3]
    $aWin[$iElement][0] = WinGetHandle ($sWindowText)
    $aWin[$iElement][1] = $sWindowText
    $aWin[$iElement][2] = $VendorId
EndFunc

Func observe(Byref $aWin)
  Local $hWnd, $lieferant
  While True
    For $i = 0 to UBound($aWin) - 1  
        $hWnd = $aWin[$i][0]
        If $hWnd Then
            If WinExists($hWnd) And WinGetTitle($hWnd) == $aWin[$i][1] Then
                $lieferant = getLieferant($hWnd) ; Pass valid hwnd onto getLieferant have it return the vendorID & store it
                If $lieferant = "" Then
                    ;VendorID not found..
                    ;Delete window from list? 
                    ;Check again later?
                    ;Throw Warning?
                ElseIf $lieferant <> $aWin[$i][2] Then
                    ConsoleWrite("NEW " & $lieferant & @crlf)
                    refreshSidebar($lieferant)
                    $aWin[$i][2] = $lieferant
                EndIf
            Else
                $aWin[$i][0] = 0 ; Win No longer exists donot check again
            EndIf
        EndIf
    Next
    Sleep(250)  
  WEnd
EndFunc

Func getLieferant($hWnd)
  Local $strString = WinGetText($hWnd)
  Local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
  If Not IsArray($arrResult) Then Return ""
  Return $arrResult[0]
EndFunc

 

Edited September 17, 2019 by Bilgus

[water]

@keeeniiic

There is no need to quote a post for your answer (except when you want to refer to a specific part of a previous post - then mark this part and click on the appearing "Quote selection") . We know what we have written
Simply enter your reply in the input field at the end of the thread and click "Submit Reply"

[water]

I would - in addition - suggest a hotkey to exit the script. Else such code would make your script wait forever when there isn't and never will be a window named "Test2 Rechnungskontrolle (Verbuchung)".

While Not WinExists("Test2 Rechnungskontrolle (Verbuchung)")
    Sleep(250)
WEnd

 

[Bilgus]

31 minutes ago, water said:

I would - in addition - suggest a hotkey to exit the script. Else such code would make your script wait forever when there isn't and never will be a window named "Test2 Rechnungskontrolle (Verbuchung)"

 

or just keep count of how many windows succeeded..

Func observe(Byref $aWin)
  Local $iValid = 1, $hWnd, $lieferant
  While $iValid > 0
    $iValid = 0
    For $i = 0 to UBound($aWin) - 1  
        $hWnd = $aWin[$i][0]
        If $hWnd Then
            If WinExists($hWnd) And WinGetTitle($hWnd) == $aWin[$i][1] Then
                ...
                $iValid += 1
            Else
                $aWin[$i][0] = 0 ; Win No longer exists donot check again
            EndIf
        EndIf
    Next
    Sleep(250)  
  WEnd
EndFunc

 

[keeeniiic]

@Bilgus thank you for the suggestion

@water thank you for the hint, won't happen again.

Thank you for the last answers, would this code cover, if the user reopens the window, because this happens quite often, they work in the "Test 2 ... " window then close it open another one and later come back to the "Test 2.. " window.

Let me know thanks

[Bilgus]

I don't know enough about your application if the hWnd stays the same then sure but generally when closing and reopening windows the hwnd changes so you might have to search for it by name each time its not found...

 

The code I posted will still work but it'll need to be tweaked

 

 

[Bilgus]

expandcollapse popup

Opt("MustDeclareVars", 1)

Global $lieferant = 0 ; make it to a value that cannot be found
Global $aWindows[0][3] ;Hwnd, WinText, last vendorID
Global $bQuit = False

HotKeySet("+!q", "Quit_Loop") ; Shift-Alt-q
HotKeySet("+!a", "AddCurrentWindow") ; Shift-Alt-a

AddWindow($aWindows, "Test2 Rechnungskontrolle (Verbuchung)")
observe($aWindows)

Func AddWindow(Byref $aWin, $sWindowText, $VendorId = 0)
    Local $iElement = UBound($aWin, 1)
    ReDim $aWin[$iElement + 1][3]
    $aWin[$iElement][0] = WinGetHandle ($sWindowText)
    $aWin[$iElement][1] = $sWindowText
    $aWin[$iElement][2] = $VendorId
EndFunc

Func observe(Byref $aWin)
  Local $hWnd, $lieferant
  While Not $bQuit
    For $i = 0 to UBound($aWin) - 1  
        $hWnd = $aWin[$i][0]
        If $hWnd Then
            If WinExists($hWnd) And WinGetTitle($hWnd) == $aWin[$i][1] Then
                $lieferant = getLieferant($hWnd) ; Pass valid hwnd onto getLieferant have it return the vendorID & store it
                If $lieferant < 0 Then
                    ;VendorID not found..
                    ;Delete wiondow from list? 
                    ;Check again later?
                    ;Throw Warning?
                ElseIf $lieferant <> $aWin[$i][2] Then
                    ConsoleWrite("NEW " & $lieferant & @crlf)
                    refreshSidebar($lieferant)
                    $aWin[$i][2] = $lieferant
                EndIf
            Else
                $aWin[$i][0] = WinGetHandle ($aWin[$i][1])
            EndIf
        Else
            ;Try to find the window again   
            $aWin[$i][0] = WinGetHandle ($aWin[$i][1])
        EndIf
    Next
    Sleep(250)  
  WEnd
EndFunc

Func getLieferant($hWnd)
  Local $strString = WinGetText($hWnd)
  Local $arrResult = StringRegExp($strString, '(?s)&Adresse.*?\d+.*?(\d+)', $STR_REGEXPARRAYGLOBALMATCH)
  If Not IsArray($arrResult) Then Return -1
  Return $arrResult[0]
EndFunc

Func refreshSidebar($vendorID)
  ; Initialize and get session handle
  Local $hOpen = _WinHttpOpen()
  ; Get connection handle
  Local $hConnect = _WinHttpConnect($hOpen, "http://MKSRVAPP04:25024")

  Local $hRequest = _WinHttpSimpleRequest($hConnect, "GET", "api/client/sidebar/sendCommand?user=MULTIKRAFT\" & @UserName & "&call=OBJ:Vendor@@SELECT@@IDX:Vendor%20ID=" & $vendorID)

  ; Close handles
  _WinHttpCloseHandle($hConnect)
  _WinHttpCloseHandle($hOpen)
EndFunc   ;==>refreshSidebar

Func Quit_Loop()
    $bQuit = True
EndFunc

Func AddCurrentWindow()
    Local $hWnd = WinGetHandle("[ACTIVE]", "")
    AddWindow($aWindows, WinGetText($hWnd))
    ConsoleWrite("Added: " & WinGetTitle($hWnd) & @crlf)
EndFunc

And Hotkeyset is indeed your best bet since there could be no active windows for a bit