How make proc more effective?

Question

There is a collection of vertices: [ x1 y1 x2 y2 .. xn yn ]

I would like to change the center of those coordinates. So I try:

proc changeCenter { vertices X Y } {
    set myList [list]
    foreach element $vertices {
        lappend myList [expr [lindex $element 0] + $X]
        lappend myList [expr [lindex $element 1] + $Y]
    }

 return $myList 
}

But its performance too slow.

How can I change above code to be more effective or maybe need to change the representation of vertices?

Answer 1

The simplest way to do that is by bracing your expression:

proc changeCenter { vertices X Y } {
    set myList [list]
    foreach {x0 y0} $vertices {
        lappend myList [expr {$x0 + $X}]
        lappend myList [expr {$y0 + $Y}]
    }

    return $myList 
}

This has several benefits:

• The numbers don't have to be converted to a string and back.
• The expressions can be compiled.
• You don't get nasty rounding errors introduced by the string representation.
• It prevents injection of code that you don't want to execute.

See this wiki entry for further details.

And use foreach with more than one variable if you don't use nested lists as input.

Answer 2

Your changeCenter proc indicates the vertices collection is a list of pairs ({{x1 y1} {x2 y2} ...}), yet you are returning a flat list:

proc changeCenter { vertices deltaX deltaY } {
    set recentered [list]
    foreach vertex $vertices {
        lassign $vertex x y
        lappend recentered [list [expr {$x + $deltaX}] [expr {$y + $deltaY}]]
    }
    return $recentered 
}

If the vertices really are a flat list ({x1 y1 x2 y2 ...}) then read the list 2 elements at a time:

proc changeCenter { vertices deltaX deltaY } {
    set recentered [list]
    foreach {x y} $vertices {
        lappend recentered [expr {$x + $deltaX}] [expr {$y + $deltaY}]
    }
    return $recentered 
}

---

I haven't benchmarked it, but I suspect updating the vertices list in-place might be faster than appending to a new list:

proc changeCenter { vertices deltaX deltaY } {
    for {set i 0} {$i < [llength $vertices]} {incr i} {
        lset vertices $i 0 [expr {[lindex $vertices $i 0] + $deltaX}] 
        lset vertices $i 1 [expr {[lindex $vertices $i 1] + $deltaY}] 
    }
    return $vertices 
}

or

proc changeCenter { vertices deltaX deltaY } {
    for {set i 0} {$i < [llength $vertices]} {incr i 2} {
        lset vertices $i [expr {[lindex $vertices $i] + $deltaX}] 
        set j [expr {$i + 1}]
        lset vertices $j [expr {[lindex $vertices $j] + $deltaY}] 
    }
    return $vertices
}

depending on the structure of the vertices list as mentioned above.

---

Passing the vertices list by name would be faster still (avoid copying the data):

proc changeCenter { verticesName deltaX deltaY } {
    upvar 1 $verticesName v
    for {set i 0} {$i < [llength $v]} {incr i 2} {
        lset v $i [expr {[lindex $v $i] + $deltaX}] 
        set j [expr {$i + 1}]
        lset v $j [expr {[lindex $v $j] + $deltaY}] 
    }
    # no need to return a value
}

and call it with the variable name:

changeCenter vertices 1 2