foreach loop - window name exists and expected boolean value error

0
votes

Morning all,

I am running the below code

foreach rs_lb {kl15_lb din1_lb din2_lb din3_lb din4_lb \
               dinnc_lb ain1_lb ain2_lb ain3_lb ain4_lb a_lb \
               b_lb u_lb v_lb w_lb sin_lb cos_lb th1_lb th2_lb hvil_lb} \

        rs_lb_txt {KL15 DIN1 DIN2 DIN3 DIN4 DINNC AIN1 AIN2 AIN3 AIN4 \
        A B U V W SIN COS TH1 TH2 HVIL} \

        rs_lb_rw {0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9} \

        rs_lb_cm {0 0 0 0 0 0 0 0 0 0 2 2 2 2 2 2 2 2 2 2} \

        rs_cb {kl15_cb din1_cb din2_cb din3_cb din4_cb dinnc_cb ain1_cb \
        ain2_cb ain3_cb ain4_cb a_cb \
        b_cb u_cb v_cb w_cb sin_cb cos_cb th1_cb th2_cb hvil_cb} \

        rs_cb_rw {0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9} \

        rs_cb_cm {1 1 1 1 1 1 1 1 1 1 3 3 3 3 3 3 3 3 3 3} \

        ds_out_i {0 0 2 0 4 0 6 0 8 0 10 0 12 0 14 0 16 0 18 0 20 0 \
                  22 0 24 0 26 0 28 0 30 0 32 0 34 0 36 0 38 0} \
{
    label .rs.$rs_lb -text "$rs_lb_txt"     

    checkbutton .rs.$rs_cb -variable $rs_cb -command {
        if {$rs_cb} {
            set ds_out($ds_out_i) 1
            set ds_out([expr $ds_out_i + 1]) 1
        } else {
            set ds_out($ds_out_i) 0
            set ds_out([expr $ds_out_i + 1]) 0
        }
    }
    grid .rs.$rs_lb -row $rs_lb_rw -column $rs_lb_cm    
    grid .rs.$rs_cb -row $rs_cb_rw -column $rs_cb_cm
}

and I'm getting the error:

window name "" already exists in parent

and when I click on a checkbox I get the application error:

expected boolean value but got "" expected boolean value but got "" while executing "if {$rs_cb} { set ds_out($ds_out_i) 1 set ds_out([expr $ds_out_i + 1]) 1 } else { set ds_out($ds_out_i) 0 set ds_out([expr $ds_out_i + ..." invoked from within ".rs.u_cb invoke" ("uplevel" body line 1) invoked from within "uplevel #0 [list $w invoke]" (procedure "tk::ButtonUp" line 24) invoked from within "tk::ButtonUp .rs.u_cb" (command bound to event)

Can anyone tell me why this is happening please?

2
tcltk
Your code was hard to read (before I cleaned it up by using more conventional indentation). That's leading you into much trouble, as it makes it easy to miss that you're doing other bad practices. – Donal Fellows
Hi Donal, thanks for your help. You are right that I'm using bad practices, I just don't know what good practises are. – Kenny Barber

2 Answers

2
votes

The window name "" already exists in parent is because the list feeding ds_out_i is longer (twice as long!) than the lists feeding all the other variables; foreach keeps going until it has gone over all the elements for each item, assigning the variables whose lists have been depleted to the empty string. I'm guessing that you want to either use a double-variable list iteration for the list currently feeding ds_out_i or, more likely, iterate over a list of pairs. (Note that this breaks your code in your callback a bit more; I'll get to that in a moment.)

    …
    ds_out_i {{0 0} {2 0} {4 0} {6 0} {8 0} {10 0} {12 0} {14 0} {16 0} {18 0} {20 0} \
              {22 0} {24 0} {26 0} {28 0} {30 0} {32 0} {34 0} {36 0} {38 0}} \
    …

Your other error is because you're currently using a complex embedded script for the -command option (to checkbutton). Don't do that! It's really hard to work with. The very strongly recommended approach is to make a little helper procedure and then make the -command option be a simple script that calls that procedure and which is generated with the list command.

Do Not Use Complex Scripts Directly In Callbacks

We mean it. If I could make that a flashing banner, I would. It's very difficult to get right, and so easy to get wrong. Using helper procedures is hugely easier to get right. Here's how that might look for you.

proc rs_checkbutton_callback {varname index} {
    # "parse" the arguments
    global ds_out;             # We're working with a global variable here
    upvar "#0" $varname rs_cb; # Alias the named global variable as rs_cb
    lassign $index a b;        # This splits the two-part ds_out_i into two variables, a and b

    if {$rs_cb} {
        set ds_out($a) 1
        set ds_out($b) 1
    } else {
        set ds_out($a) 0
        set ds_out($b) 0
    }
}

…

checkbutton .rs.$rs_cb -variable $rs_cb \
    -command [list rs_checkbutton_callback $rs_cb $ds_out_i]

I'm not sure if that callback procedure is doing what you really want. It might instead have been better off being done as:

proc rs_checkbutton_callback {varname index} {
    # "parse" the arguments
    global ds_out;             # We're working with a global variable here
    upvar "#0" $varname rs_cb; # Alias the named global variable as rs_cb
    lassign $index a b;        # This splits the two-part ds_out_i into two variables, a and b

    if {$rs_cb} {
        set ds_out([list $a $b]) 1
        set ds_out([list $a [expr {$b + 1}]) 1
    } else {
        set ds_out([list $a $b]) 0
        set ds_out([list $a [expr {$b + 1}]) 0
    }
}
0
votes

One good programming principle is Occam's Razor, i.e. "Entities are not to be multiplied without necessity". You can create the widget paths and the checkbox labels from a simple list of names:

foreach name {foo bar baz} {
    lappend lbnames ${name}_lb
    lappend cbnames ${name}_cb
    lappend texts   [string toupper $name]
}
list $lbnames $cbnames $texts
# => {foo_lb bar_lb baz_lb} {foo_cb bar_cb baz_cb} {FOO BAR BAZ}

In this case, it is a good idea to split the name list into two equal parts:

set names1 {
    kl15 din1 din2 din3 din4 dinnc ain1 ain2 ain3 ain4
}

set names2 {
    a b u v w sin cos th1 th2 hvil
}

Then we can start creating widgets. Unfortunately, I can't figure out how you want the command to work, but it would probably be easy to integrate the callback invocation here as well. Since we create the columns of widgets in parallel, we don't need to keep track of grid rows and columns:

toplevel .rs

foreach name1 $names1 name2 $names2 {
    grid \
        [label .rs.${name1}_lb -text [string toupper $name1]] \
        [checkbutton .rs.${name1}_cb -variable ${name1}_cb -command [list ...]] \
        [label .rs.${name2}_lb -text [string toupper $name2]] \
        [checkbutton .rs.${name2}_cb -variable ${name2}_cb -command [list ...]]
}

This way, setting up the widget array becomes much easier and the code has more clarity.

BTW, did you know that you can set a label in the checkbutton widget?

foreach name1 $names1 name2 $names2 {
    grid \
        [checkbutton .rs.${name1}_cb -text [string toupper $name1] -variable ${name1}_cb -command [list ...]] \
        [checkbutton .rs.${name2}_cb -text [string toupper $name2] -variable ${name2}_cb -command [list ...]] \
        -sticky w
}