When Something Deserves to Be a Procedure

Part I · Module 10 · Chapter 10.10 · When Something Deserves to Be a Procedure

1. Three tests

Ask these in order. One yes is enough.

1. Is it used in more than one place?
2. Does it need a name before a reader can understand it?
3. Does it need to be checkable on its own?

Everything below is those three, each shown once where the answer is yes and once where it is no.

2. Test one: used in more than one place

The clearest case. Here the same four lines appear twice:

set ports_a {{clk 1} {data 32}}
set ports_b {{rst 1} {addr 16} {en 1}}

set n 0
foreach p $ports_a { lassign $p name width ; if {$width > 1} { incr n } }
puts "A: $n wide"

set n 0
foreach p $ports_b { lassign $p name width ; if {$width > 1} { incr n } }
puts "B: $n wide"

A: 1 wide
B: 1 wide

Extracting removes the duplicate and gives one place to fix when the rule changes:

proc count_wide {ports} {
    set n 0
    foreach p $ports {
        lassign $p name width
        if {$width > 1} { incr n }
    }
    return $n
}

puts "A: [count_wide $ports_a] wide"
puts "B: [count_wide $ports_b] wide"

A: 1 wide
B: 1 wide

Where the answer is no: a three-line calculation that appears exactly once, in the only procedure that could ever want it. Extracting it there buys nothing and costs a jump.

3. Test two: it needs a name to be understandable

Sometimes code is used once and still wants extracting, because the name is the explanation:

set w 12

if {$w > 0 && ($w & ($w - 1)) == 0} {
    puts "ok"
} else {
    puts "rejected"
}

rejected

($w & ($w - 1)) == 0 is a known trick and it is not self-evident. A name says what it is:

proc is_power_of_two {n} {
    return [expr {$n > 0 && ($n & ($n - 1)) == 0}]
}

foreach w {8 12 1 0} {
    puts "$w -> [expr {[is_power_of_two $w] ? "ok" : "rejected"}]"
}

8 -> ok
12 -> rejected
1 -> ok
0 -> rejected

The condition is now readable at the call site without understanding the bit trick, and the trick itself is in one place with a name that says what it does.

Where the answer is no: expr {$total / $count} does not become clearer as proc divide {a b}. A name helps when it replaces something opaque; wrapping something already obvious just adds a layer.

4. Test three: it needs to be checkable on its own

The criterion people most often skip. Some logic is worth isolating simply so you can try it against known inputs:

proc parse_setting {line} {
    set t [string trim $line]
    if {$t eq ""}                 { return "" }
    if {[string match "#*" $t]}   { return "" }
    if {![string match "*=*" $t]} { error "not a setting: '$t'" }
    set i [string first "=" $t]
    return [list [string trim [string range $t 0 [expr {$i - 1}]]] \
                 [string trim [string range $t [expr {$i + 1}] end]]]
}

Because it is a procedure taking a value and returning a value, a check is four lines:

set cases {
    {"width = 32"   {width 32}}
    {"  a=b  "      {a b}}
    {"# comment"    {}}
    {""             {}}
}

set failures 0
foreach case $cases {
    lassign $case input expected
    set got [parse_setting $input]
    if {$got ne $expected} {
        puts "  FAIL '$input': expected '$expected', got '$got'"
        incr failures
    }
}
puts "[llength $cases] cases, $failures failure(s)"

4 cases, 0 failure(s)

That is only possible because the parsing is separable from the loop that reads lines and the code that acts on them. Had it stayed inline, testing it would mean constructing a file.

Where the answer is no: code with no interesting behaviour to check — a loop that prints a formatted table has one correct output and you verify it by looking at it.

5. Signals that are not criteria

Three things that feel like reasons and are not.

Length. A twenty-line procedure doing one coherent job is fine. A four-line one that is called from six places earns its place. Line count correlates with nothing.

"It might be reused later." Speculative extraction produces procedures whose interfaces were designed for a caller that never arrived. Extract when the second caller exists, not when you imagine it.

Nesting depth alone. Chapter 9.10 §5 recommended extracting a deeply nested inner loop — but the reason given there was that you could name what it does, which is test two. Depth is the symptom; the name is the criterion.

6. What extraction costs

The part usually left out. Pulling code into a procedure is not free.

It separates the code from its context. A reader now has to jump. If the fragment only makes sense alongside the five lines around it, the jump costs more than the indentation saved.

It fixes an interface. Once something is a procedure, its parameter list is a contract. Changing it means visiting every caller — which is the whole point when there are many, and pure overhead when there is one.

It can leak the wrong abstraction. A procedure extracted from a single call site tends to take exactly the arguments that call site had, in that order, including ones it does not need:

# extracted too eagerly — `label` is not used at all
proc count_wide_bad {ports label threshold} {
    set n 0
    foreach p $ports {
        lassign $p name width
        if {$width > $threshold} { incr n }
    }
    return $n
}
puts [count_wide_bad {{clk 1} {data 32}} "unused" 1]

1

A second caller then either passes a meaningless label or forces a redesign. Waiting for the second caller would have shown which parameters were real.

7. A worked judgement

One script, four candidates, four different answers:

proc is_power_of_two {n} {
    return [expr {$n > 0 && ($n & ($n - 1)) == 0}]
}

proc count_wide {ports} {
    set n 0
    foreach p $ports {
        lassign $p name width
        if {$width > 1} { incr n }
    }
    return $n
}

set designs {
    {alpha {{clk 1} {data 32}}}
    {beta  {{rst 1} {addr 12} {en 1}}}
}

foreach d $designs {
    lassign $d name ports

    # candidate A: the wide-port count — extracted (used twice below)
    set wide [count_wide $ports]

    # candidate B: the power-of-two check — extracted (needs a name)
    set widths {}
    foreach p $ports {
        lassign $p pname w
        if {![is_power_of_two $w]} { lappend widths $pname }
    }

    # candidate C: this format call — left inline (obvious, used once)
    puts [format "  %-6s %d wide, %d total" $name $wide [llength $ports]]

    # candidate D: this message — left inline (only makes sense here)
    if {[llength $widths] > 0} {
        puts "         non-power-of-two: [join $widths {, }]"
    }
}

puts "totals: [count_wide {{a 1} {b 8}}] of 2"

  alpha  1 wide, 2 total
  beta   1 wide, 3 total
         non-power-of-two: addr
totals: 1 of 2

Two extracted, two not — and the reasons are different in each pair. count_wide passes test one; is_power_of_two passes test two; the format call and the message pass none.

8. What Module 10 covered

Ten chapters on turning a fragment into a command:

• proc as an ordinary command that registers another command, and the implicit result rule
• Argument counts, and the generated usage line as an always-accurate contract
• Defaults, including the two places Tcl does not check what you wrote
• args and the {*}$args forwarding idiom
• return, and giving every path an explicit one
• Local variables — and the asymmetry that commands are global while variables are not
• Guard clauses, with the choice between returning a neutral value and raising
• Returning lists and dictionaries instead of writing to globals
• rename, for wrapping anything — and why a stray redefinition is invisible
• When to extract, as three tests rather than an instinct

The thread is the one Chapter 10.1 opened with: proc is a command, not syntax. Your procedures go in the same flat table as the built-ins, which is why they can be renamed, wrapped, redefined and inspected — and why a duplicate definition wins silently. Almost every surprise in the module is that single fact, or its counterpart: variables, unlike commands, are private to a frame.

9. What comes next

Module 11 is Scope and Namespaces, and it settles three debts this module deliberately left open:

• upvar, promised twice — in Chapter 7.1 §6 as the only way to pass an array to a procedure, and in Chapter 10.8 §7 as the out-parameter alternative.
• uplevel, for running a script in the caller's frame.
• More than two levels of scope. Chapter 10.6 assumed global and local are all there is; namespaces are the rest of the answer.

Nothing in this module becomes wrong. Module 11 adds to it.

10. Exercises

Exercise 1. State the three tests.

Exercise 2. For each, say extract or leave, and which test decides it: (a) a four-line calculation appearing in two procedures; (b) expr {$a + $b} used once; (c) a bit-manipulation condition nobody can read; (d) a puts of a formatted line, used once.

Exercise 3. Why is "it might be reused later" a bad reason to extract?

Exercise 4. Name three costs of extraction.

Exercise 5. Extract the readable piece from this and leave the rest:

foreach f $files {
    set ext [string tolower [file extension $f]]
    if {$ext in {.v .sv .vhd}} {
        puts "  design file: $f"
    }
}

Exercise 6. What does it mean, practically, for a fragment to be "checkable on its own"?

Answers

1. Is it used in more than one place? Does it need a name before a reader can understand it? Does it need to be checkable on its own?

2. (a) extract — test one; (b) leave — it passes none, and a name would not make addition clearer; (c) extract — test two, the name replaces something opaque; (d) leave — used once, obvious, and only meaningful in context.

3. Because the interface gets designed for an imagined caller rather than a real one. When the second caller eventually arrives it usually wants something slightly different, and you are left either reworking the procedure or bending the new caller to fit. Waiting until there are two shows you what actually varies.

4. It separates code from its context so a reader must jump; it fixes an interface that then costs something to change; and extracted-too-early procedures tend to carry parameters the single original caller happened to have, including unused ones.

5.

proc is_design_file {path} {
    return [expr {[string tolower [file extension $path]] in {.v .sv .vhd}}]
}

foreach f $files {
    if {[is_design_file $f]} {
        puts "  design file: $f"
    }
}

The classification wants a name — it is a rule with a meaning, and it will plausibly be needed elsewhere. The loop and the puts stay: they are one line each and only mean anything here.

6. That you can call it with known inputs and compare the result with a known output, without building a file, setting a global, or arranging any other state first. If that is possible, the logic is separable from its surroundings — which is worth something whether or not a check is ever written.

11. Remember

• Three tests, one yes is enough: used in more than one place; needs a name to be understandable; needs to be checkable on its own.
• Two occurrences is enough — by the second you know what varies.
• A name earns its place when it replaces something opaque, not when it wraps something already obvious.
• "Checkable on its own" means callable with known inputs and no set-up.
• Length, nesting depth and possible future reuse are not criteria. Depth is a symptom; the name is the test.
• Extraction costs: a jump for the reader, a fixed interface, and the risk of a wrong abstraction built around one call site.
• If you cannot name a fragment without describing where it is called from, leave it where it is.