samfisherirl / UIAViewer.ahk-for-UIAutomation.ahk

UIAViewer.ahk for UIAutomation.ahk, with some modifications
10 stars 1 forks source link

try...catch misuse #4

Open Descolada opened 1 year ago

Descolada commented 1 year ago

Take for example

  loop, 10
  {
  try {
el := el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2)
       break
   } catch e{
      Sleep, 100
     }
  }

This doesn't do what you think it would do. el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2) does not throw an error in AHK v1, instead it returns an empty string. This means el := el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2) sets el to an empty string, and the next 9 loops will do nothing. Instead you should use WaitElementExist with timeout of 1 second.

Similarly with this:

  try {
el.Click()  
       break
   } catch e{
      Sleep, 100
     }

el.Click() doesn't throw an error if the click doesn't happen or the element doesn't exist. This code will always break from the loop at the first iteration.

Example:

Loop 10
{
    try {
        "".Click()
        MsgBox Clicked successfully
        break
    } catch e {
        MsgBox Error
    }
}
samfisherirl commented 1 year ago

I see that now, Im unsure why I was getting error throws before, or what gave me the impression this was an effective way to accomplish this. Ill remove and default waitelementexist timeout

samfisherirl commented 1 year ago

Take for example

  loop, 10
  {
  try {
el := el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2)
       break
   } catch e{
      Sleep, 100
     }
  }

This doesn't do what you think it would do. el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2) does not throw an error in AHK v1, instead it returns an empty string. This means el := el.FindFirstBy("ControlType=TabItem AND Name='Macro creator'",,2) sets el to an empty string, and the next 9 loops will do nothing. Instead you should use WaitElementExist with timeout of 1 second.

Similarly with this:

  try {
el.Click()  
       break
   } catch e{
      Sleep, 100
     }

el.Click() doesn't throw an error if the click doesn't happen or the element doesn't exist. This code will always break from the loop at the first iteration.

Example:

Loop 10
{
    try {
        "".Click()
        MsgBox Clicked successfully
        break
    } catch e {
        MsgBox Error
    }
}

thinking more about it, it may be best to scrap changes with this much of a defect and take small steps. I now know the code inside and out, I think I can redo it more effectively.