This code can be more short?

[GodlessSinner]

Case $msg = $z[1]
            DeleteMenuItem(1)
        Case $msg =  $z[2]
            DeleteMenuItem(2)
        Case $msg = $z[3]
            DeleteMenuItem(3)
        Case $msg = $z[4]
            DeleteMenuItem(4)
        Case $msg = $z[5]
            DeleteMenuItem(5)
        Case $msg = $z[6]
            DeleteMenuItem(6)
        Case $msg = $z[7]
            DeleteMenuItem(7)
        Case $msg = $z[8]
            DeleteMenuItem(8)
        Case $msg = $z[9]
            DeleteMenuItem(9)

How to do it more short?

[Authenticity]

You can use a For loop. It won't make it faster in my opinion but just shorter.

For $i = 1 To UBound($z)-1
     If $msg = $z[$i] Then
          DeleteMenuItem($i)
          ExitLoop
     EndIf
Next

[martin]

You can use a For loop. It won't make it faster in my opinion but just shorter.

For $i = 1 To UBound($z)-1
     If $msg = $z[$i] Then
          DeleteMenuItem($i)
          ExitLoop
     EndIf
Next

It might be worth pointing out that the loop needs to be outside of the select

$Msg = GuiGetMsg()

For $i = 1 To UBound($z)-1
     If $msg = $z[$i] Then
          DeleteMenuItem($i)
          ExitLoop
     EndIf
Next

Select
  Case ...


EndSelect

[GodlessSinner]

It might be worth pointing out that the loop needs to be outside of the select

- I'm already understood it:), thanks.

[smashly]

It might be worth pointing out that the loop needs to be outside of the select

$Msg = GuiGetMsg()

For $i = 1 To UBound($z)-1
     If $msg = $z[$i] Then
          DeleteMenuItem($i)
          ExitLoop
     EndIf
Next

Select
  Case ...


EndSelect

You could put it in Select as a Case Else

$msg = GUIGetMsg()
    Select
        Case $msg = $GUI_EVENT_CLOSE
            Exit
        Case $msg = $AnotherControl
            ;;;;
        Case $msg = $Whatever
            ;;;;
        Case Else
            For $i = 1 To UBound($z) - 1
                If $msg = $z[$i] Then
                    DeleteMenuItem($i)
                    ExitLoop
                EndIf
            Next
    EndSelect

[martin]

You could put it in Select as a Case Else...]

Yes, that's a neater way.

[GodlessSinner]

Good idea, Smashly, thanks.

[Nutster]

A slightly more compact way to do this would be to use the switch statement instead of the select statement.

$msg = GUIGetMsg()
Switch $msg
Case $GUI_EVENT_CLOSE
     Exit
Case $AnotherControl
     ;;;;
Case $Whatever
     ;;;;
Case Else
     For $i = 1 To UBound($z) - 1
          If $msg = $z[$i] Then
               DeleteMenuItem($i)
               ExitLoop
          EndIf
     Next
EndSwitch

This way AutoIt only has to evaluate $msg once, not each time, well until you get into the loop at the end. Also, it does not keep restating the $msg = part each time, so less typing.

[GodlessSinner]

Question solved, thanks to all. I know that the next words will be not in topic, but what's wrong with forum? :

This code can be more short? Godless 8 Replies 0 Views

Edited August 16, 2009 by Godless

[martin]

A slightly more compact way to do this would be to use the switch statement instead of the select statement.

$msg = GUIGetMsg()
Switch $msg
Case $GUI_EVENT_CLOSE
     Exit
Case $AnotherControl
     ;;;;
Case $Whatever
     ;;;;
Case Else
     For $i = 1 To UBound($z) - 1
          If $msg = $z[$i] Then
               DeleteMenuItem($i)
               ExitLoop
          EndIf
     Next
EndSwitch

This way AutoIt only has to evaluate $msg once, not each time, well until you get into the loop at the end. Also, it does not keep restating the $msg = part each time, so less typing.

$msg does has to be evaluated for every case whether it is a switch or a select and the speed difference is negligible. I recon about 1 ms extra for a Select compared to a Switch regardless of the number of Cases, and when you use GuiGetMsg more than 99% of the time is used by GuiGetMsg anyway. Also Select can have conditions in some Case tests which cannot be dealt with in a Switch so it depends on what else the OP is using in his Select.

[Authenticity]

I think that the meaning is that AutoIt is interpreted language and thus re-evaluating the string MSG and what it's purpose in the code (or it's value) is not a wise approach to be handled in a Select...EndSelect conditional structure, if it's possible to avoid doing so it's preferred. Not sure though.

[martin]

I think that the meaning is that AutoIt is interpreted language and thus re-evaluating the string MSG and what it's purpose in the code (or it's value) is not a wise approach to be handled in a Select...EndSelect conditional structure, if it's possible to avoid doing so it's preferred. Not sure though.

You might be right, but my logic was that even in Switch statements a Case $Value can only be evaluated by comparing $value against the switch value so it has to be re-evaluated and it might not be any different to using Select /Case $msg = $Value. So I made a test script and timed a Switch with 7 different Cases, and a Select with the same equivalent cases. I found that for 1000 loops of Switch it was 1 second faster than 1000 cases of Select even if I added extra Cases to both. It took 9 seconds for 100 loops of switch and 10 seconds for select. That is where I got my 1 mS from.

If I changed the GuiGetMsg() for a constant the times becameapproximately 1000 times faster with no obvious differencein speed between the 2 methods.

So my conclusion is that it is purely up to however you want to do it, and the benefits of one over the other are minor except when you have to use Select because of the logical tests you want to make.

[Valuater]

You ca even cut out the first $Msg by using this

Switch GUIGetMsg()

8)

[Nutster]

Internally, the Switch argument is only evaluated once, at the very beginning. That value is then stored. That value does not need to be evaluated again. Only the Case values are evaluated for each case and compared to the value that was stored at the beginning. If I recall, the execution savings between Switch and Select is about 5%, not much but it adds up through multiple loops calling the switch each time.