5
\$\begingroup\$

I had the following three Tcl procedures, which calculate positions along three edges of a rectangle:

#---- Numbered pins at the top ---------------------------------------
# Given a list of triplets (number, name, index), place the pins for the
# signals "name<index>" along the top edge, positioned vertically at 'y'
# and horizontally at 'number' times a constant space left of 'xRight'
# (plus an arbitrary offset of 2).
proc setNumberedTopPins {xRight y pins} {
    set padWidth 95
    foreach {number name index} $pins {
        set xOffset [expr $number * $padWidth + 2]
        setPin $name [expr $xRight-$xOffset] $y Top $index
    }
}
#---- Numbered pins at the right -------------------------------------
# The same for the right edge.
proc setNumberedRightPins {x yTop pins} {
    set padWidth 95
    foreach {number name index} $pins {
        set yOffset [expr max(0, $number * $padWidth - 2.5)]
        setPin $name $x [expr $yTop-$yOffset] Right $index
    }
}
#---- Numbered pins at the bottom ------------------------------------
# The same for the bottom edge.
proc setNumberedBottomPins {xRight y pins} {
    set padWidth 95
    foreach {number name index} $pins {
        set xOffset [expr $number * $padWidth + 2]
        setPin $name [expr $xRight-$xOffset] $y Bottom $index
    }
}

Since the code for each of the procedures is almost identical, I decided to write a single generalized procedure and three specializations of it.

I had some difficulties with generalizing the fact that the offset is sometimes applied to the second (x) and sometimes to the third (y) argument of setPin. I wanted to make sure that this decision is only taken once, outside the foreach loop.

Here is the solution I came up with:

#---- Numbered pins at the top ---------------------------------------
proc setNumberedTopPins {xRight y pins} {
    setNumberedPins Top 2 $xRight $y $pins
}
#---- Numbered pins at the right -------------------------------------
proc setNumberedRightPins {x yTop pins} {
    setNumberedPins Right -2.5 $x $yTop $pins
}
#---- Numbered pins at the bottom ------------------------------------
proc setNumberedBottomPins {xRight y pins} {
    setNumberedPins Bottom 2 $xRight $y $pins
}
#---- Generic numbered pins ------------------------------------------
proc setNumberedPins {side offsetPin0 x y pins} {
    set padWidth 95
    switch $side {
        Top - Bottom {set coords {list [expr $x-$offset] $y}}
        Right {set coords {list $x [expr $y-$offset]}}
    }
    foreach {number name index} $pins {
        set offset [expr max(0, $number * $padWidth + $offsetPin0)]
        setPin $name {*}[eval $coords] $side $index
    }
}

I'm not an experienced Tcl programmer and I wonder if there is a better way to solve this. Particularly, there is still some amount of code duplication left (almost identical definition of auxiliary variable coords inside switch) and I would like to eliminate that (while keeping the code reasonably readable).

\$\endgroup\$
0

2 Answers 2

3
\$\begingroup\$

I would suggest that since only 2 of the procs are close enough to identical, just "combine" those:

set padWidth 95

proc setNumberedRightPins {x yTop pins} {
    foreach {number name index} $pins {
        set yOffset [expr {max(0, $number * $::padWidth - 2.5)}]
        setPin $name $x [expr {$yTop-$yOffset}] Right $index
    }
}

proc setNumberedTopPins {args} {
    setNumberedTopOrBottomPins {*}$args Top
}

proc setNumberedBottomPins {args} {
    setNumberedTopOrBottomPins {*}$args Bottom
}

proc setNumberedTopOrBottomPins {xRight y pins side} {
    foreach {number name index} $pins {
        set xOffset [expr {$number * $::padWidth + 2}]
        setPin $name [expr {$xRight-$xOffset}] $y $side $index
    }
}

Notes:

  • always brace your expressions, gain a big performance boost
  • constants can live in the global namespace, and you access them with a fully qualified variable name
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your feedback! Things I will take from it: using the magic args argument; bracing the expressions (it's always good to have good habits, although it will not make a noticable speed difference in this case). I did know about referring to global variables with ::, but I wanted to keep the "knowledge" of the padWidth within the function. And I would rather like to generalize more, not less, so I kept the single setNumberedPins procedure. \$\endgroup\$
    – mkrieger1
    Apr 24, 2015 at 12:22
0
\$\begingroup\$

After learning about arrays here, I attempted to put them to use and now have found a version of the setNumberedPins procedure that I am happy with.

The main difference is that instead of dynamically assigning one of two possible (and very similar, thereby repeating myself) pieces of code to a variable inside switch, which is then evaluated later, I only decide which of the coordinates x or y is the constant one and which is the variable one and then using these for indexing the array.

I also put braces around the expr argument(s), as suggested by glenn jackman.

proc setNumberedPins {side offsetPin0 x y pins} {
    set padWidth 95
    switch $side {
        Top - Bottom {lassign {x y} var const}
        Right        {lassign {x y} const var}
    }
    set coords($const) [set $const]
    foreach {number name index} $pins {
        set offset [expr {max(0, $number * $padWidth + $offsetPin0)}]
        set coords($var) [expr {[set $var] - $offset}]
        setPin $name $coords(x) $coords(y) $side $index
    }
}

(Before posting the code here, I thought const and var had no special meaning in Tcl, but now I am surprised that they are syntax highlighted...)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.