Is there a shorter/better way of doing this?

[mikejs]

Suppose you have a popup menu (or similar) with a bunch of entries that you want to ensure are ticked/unticked correctly based on the values of particular variables. One way of doing this would be:

Code: Select all

If var.l=#this
  SetMenuItemState(#menu, #menuitem, #True)
Else
  SetMenuItemState(#menu, #menuitem, #False)
EndIf

But, it occurs to me that "if var.l=#this" is a true/false matter in the first place, so in theory something along the lines of this should work, and is much shorter:

Code: Select all

SetMenuItemState(#menu, #menuitem, var.l=#this)

Except of course it doesn't, because it assigns var.l the value of #this, rather than testing for equality (and seems to return #true in the process). A hacky solution is to move the test into a procedure:

Code: Select all

Procedure.l Compare(in.l, comp.l)
  Define out.l
  If in=comp
    out=#True
  Else
    out=#False
  EndIf
  ProcedureReturn out
EndProcedure

SetMenuItemState(#menu, #menuitem, Compare(var.l, #this))

This is better, and if there are a bunch of menu entries to be dealt with, it makes the code for doing so a lot shorter and more readable (one line per menu entry, which will tick or untick the menu entry as required). The question is, whether there is a native way of testing for equality within the parameters to a procedure. Compare() feels pointless, even though I can't find a way to do without it.

Any suggestions?

[blueznl]

Uh... <scratching thick hairless scalp>

Why not simply do this?

Code: Select all

SetMenuItemState(#menu, #menuitem, var.l)

[mikejs]

Uh... <scratching thick hairless scalp>

Why not simply do this?

Code: Select all

SetMenuItemState(#menu, #menuitem, var.l)

Because var.l is not necessarily boolean. Sorry, looking at my earlier posting I think I oversimplified it - in my app, var is a user-configurable option that has one of three states (all of them non-zero, as it happens), and there are three corresponding menu entries. So I can do it the long way:

Code: Select all

If opt_albumorder=#om_linear
  SetMenuItemState(#m_albumorder, #act_album_order_linear, #True)
Else
  SetMenuItemState(#m_albumorder, #act_album_order_linear, #False)
EndIf

If opt_albumorder=#om_random
  SetMenuItemState(#m_albumorder, #act_album_order_random, #True)
Else
  SetMenuItemState(#m_albumorder, #act_album_order_random, #False)
EndIf

If opt_albumorder=#om_ignore
  SetMenuItemState(#m_albumorder, #act_album_order_ignore, #True)
Else
  SetMenuItemState(#m_albumorder, #act_album_order_ignore, #False)
EndIf

Or I can do it this way:

Code: Select all

SetMenuItemState(#m_albumorder, #act_album_order_linear, Compare(opt_albumorder,#om_linear))
SetMenuItemState(#m_albumorder, #act_album_order_random, Compare(opt_albumorder,#om_random))
SetMenuItemState(#m_albumorder, #act_album_order_ignore, Compare(opt_albumorder,#om_ignore))

What I was wondering was whether there was a more built-in way of achieving the latter approach.

[DarkDragon]

Not documented (so not officially supported):

Code: Select all

SetMenuItemState(#menu, #menuitem, ((var.l=#this) Or #False))

Officially supported:

Code: Select all

Procedure.i IsEqual(A.i, B.i)
  If A = B
    ProcedureReturn #True
  EndIf
  ProcedureReturn #False
EndProcedure

SetMenuItemState(#menu, #menuitem, IsEqual(var, #this))

[skywalk]

If you are careful about your constant definitions you can use ~...
For me, I have to do other things when changing a menustate, so I can afford the extra code.
I update a structured array of menu stuff and then recreate the menu when required. (MRU lists and user state changes).

Code: Select all

#m0 = 0   ; 0000
#m1 = 1   ; 0001
#m2 = 2   ; 0010
#m3 = 3   ; 0011, this constant will require extra logic
#m4 = 4   ; 0100
Define.i ms
If OpenWindow(0, 200, 200, 200, 100, "Menu Example")
  If CreateMenu(0, WindowID(0))  
    MenuTitle("Options...")
    MenuItem(1, "#m1")
    MenuItem(2, "#m2")
    MenuItem(3, "#m3")
    MenuItem(4, "#m4")
  EndIf
  ms = 2      
  SetMenuItemState(0,1,ms & #m1)
  SetMenuItemState(0,2,ms & #m2)
  SetMenuItemState(0,3,ms & #m3)  ;<-- And(&) doesn't work here since multiple bits = '1'  
  SetMenuItemState(0,4,ms & #m4)  
  Repeat: Until WaitWindowEvent()=#PB_Event_CloseWindow
EndIf

[mikejs]

Not documented (so not officially supported):

Code: Select all

SetMenuItemState(#menu, #menuitem, ((var.l=#this) Or #False))

Ok, that's the kind of thing I had in mind - just couldn't find the right syntax for it. But if it's not an officially supported use, I think I'll stick with the separate procedure, as it makes it a bit more obvious what the code is actually doing.

Thanks all.

[blueznl]

Ugly code, but this might work:

Code: Select all

SetMenuItemState(#menu, #menuitem1, var ! 1 +1)
SetMenuItemState(#menu, #menuitem2, var ! 2 +1)
SetMenuItemState(#menu, #menuitem3, var ! 3 +1)

I'm XOR'ing the var, so if left and right are equal, the result would be 0. Then I'll add 1, as SetMenuItemState() documentation tells me it will only be set on '1', and reset on anything else.

[Tenaja]

Uh... <scratching thick hairless scalp>

Why not simply do this?

Code: Select all

SetMenuItemState(#menu, #menuitem, var.l)

This should do it.

Code: Select all

SetMenuItemState(#menu, #menuitem, (var.l <> not #this))

You need the <> not because with just =, the compiler assumes it is an assignment.

[skywalk]

This should do it.

Code: Select all

SetMenuItemState(#menu, #menuitem, (var.l <> not #this))

You need the <> not because with just =, the compiler assumes it is an assignment.

Almost... (PB complains about your placement of Not)

Code: Select all

Procedure EQ(x,y)  
  If x = y: ProcedureReturn 1: EndIf  
EndProcedure
#m0 = 0   ; 0000
#m1 = 1   ; 0001
#m2 = 2   ; 0010
;#m3 = 3  ; 0011  ; Intentionally omit odd integers
;ms & #m3  ;<-- And(&) doesn't work here since multiple bits = '1'  
#m4 = 4   ; 0100
Define.i ms
If OpenWindow(0, 200, 200, 200, 100, "Menu State Example")
  If CreateMenu(0, WindowID(0))  
    MenuTitle("Options...")
    MenuItem(1, "1")
    MenuItem(2, "2")
    MenuItem(3, "3")
    MenuItem(4, "4")
    MenuItem(5, "5")
    MenuItem(6, "6")
  EndIf
  ms = #m4
  SetMenuItemState(0,1,ms & #m1)
  SetMenuItemState(0,2,ms & #m2)
  ; 4 ways to set menu state...
  SetMenuItemState(0,3,ms & #m4)        ; PB approved
  SetMenuItemState(0,4,EQ(ms,#m4))      ; PB approved
  SetMenuItemState(0,5,(ms = #m4 Or 0)) ; boolean expression - Not PB approved 
  SetMenuItemState(0,6,(Not ms <> #m4)) ; boolean expression - Not PB approved 
  Debug ms                              ; NOTE: ms is unchanged despite expression use
  Repeat: Until WaitWindowEvent()=#PB_Event_CloseWindow
EndIf

[blueznl]

Weeeeelllll.... I guess my solution is still the only one officially supported

[skywalk]

Weeeeelllll.... I guess my solution is still the only one officially supported

Sorry, your expression never returns a '0', so the menu state is always set to checked(ON).

Code: Select all

ms = 2
Debug "ms = " + Str(ms)              
Debug "(ms = 2 Or 0) = " + Str((ms = 2 Or 0))
Debug "(Not ms <> 4) = " + Str((Not ms <> 4))
Debug "ms ! 1 + 1 = " + Str(ms ! 1 + 1)
Debug "ms ! 2 + 1 = " + Str(ms ! 2 + 1)  
Debug "ms ! 4 + 1 = " + Str(ms ! 4 + 1)  

...as SetMenuItemState() documentation tells me it will only be set on '1', and reset on anything else.

Actually, the SetMenuItemState() is set to checked(ON) with any non-zero entry, -1,+1,99,etc.

Does this need to be changed in the manual?

[Little John]

Actually, the SetMenuItemState() is set to checked(ON) with any non-zero entry, -1,+1,99,etc.

Does this need to be changed in the manual?

Yes, this needs to be changed then in the manual. Currently it reads:

The check is displayed when State equals 1, if State equals something else then the 'check mark' will not be displayed.

Regards, Little John

[blueznl]

Weeeeelllll.... I guess my solution is still the only one officially supported

Sorry, your expression never returns a '0', so the menu state is always set to checked(ON).

Oh, remove the +1 then. Then it should work. Looks even better

[skywalk]

Weeeeelllll.... I guess my solution is still the only one officially supported

Sorry, your expression never returns a '0', so the menu state is always set to checked(ON).

Oh, remove the +1 then. Then it should work. Looks even better

Nope, then you have negative logic and require a 'Not' to get the MenuState = 1.

Code: Select all

ms = 2
Debug "ms = " + Str(ms)              
Debug "(ms = 2 Or 0) = " + Str((ms = 2 Or 0))
Debug "(Not ms <> 4) = " + Str((Not ms <> 4))
Debug "ms ! 1 + 0 = " + Str(ms ! 1 + 0)
Debug "ms ! 2 + 0 = " + Str(ms ! 2 + 0)  
Debug "ms ! 4 + 0 = " + Str(ms ! 4 + 0)

On a side note, I really don't understand the statement, "...undocumented, so not supported..."
If 'Not' works in the above (dare I say) boolean examples, then hooray.
The expression is valid between the ()'s until I get a compiler error stating otherwise.
-Rant Off

[blueznl]

I should have tested

Not tested either

~(var ! 3)

Regarding rant: unsupported means it may and may not functuon, so you might end up with undetectable bugs some day... But hey, it's a choice, I admit that. If it works for you, keep using it.