Contribution by Patrick Fradin <patrick.fradin@planar.com>contrib_patrick_fradin_code_cleanup

Quoting his mail:

<pre>
==========================================================
Hi Jeff,

I spent some of my time to contribute to the TclTk community ! I'm in
late for Christmas gift but like we said in French : "Mieux vaut tard
que jamais". ;-)

I've use TclDevKit 5.3.0 tclchecker to analyse TclTk code in Tcl and
Tk library directories (library, tools and tests) to correct a lot of
warnings and few errors. (encapsulate some expr, use 'chan xxx'
instead of fconfigure, fileevent...)

I've made some improvements too :

Examples :

- Use 'lassign' instead of many 'lindex' of 'foreach/break' loop.

- Use 'in' or 'ni' operators instead of 'lsearch -exact' or to
  factorise some eq/ne && / || tests.

- Use 'eq' or 'ne' to tests strings instead of '==' or '!='.

- Use 'unset -nocomplain' to avoid 'catch {unset...}'.

- Remove some useless catch around 'destroy' calls.

- Use expand {*} instead of 'eval'. Don't touch a lot of code because
  I don't know all structs and lists. I think it could be a greater
  improvement to reduce 'eval' calls.

Due to previous experience, I dot not change any indentation ! ;-)
==========================================================
</pre>

1 files changed, 63 insertions, 64 deletions

diff --git a/tests/scrollbar.test b/tests/scrollbar.test
index 3addd28..632e489 100644
--- a/tests/scrollbar.test
+++ b/tests/scrollbar.test
@@ -11,29 +11,29 @@ package require tcltest 2.1
 eval tcltest::configure $argv
 tcltest::loadTestedCommands
 
-proc scroll args {
+proc scroll {args} {
     global scrollInfo
     set scrollInfo $args
 }
 
 proc getTroughSize {w} {
     if {[testConstraint testmetrics]} {
-	if [string match v* [$w cget -orient]] {
-	    return [expr [winfo height $w] - 2*[testmetrics cyvscroll $w]]
+        if {[string match "v*" [$w cget -orient]]} {
+	    return [expr {[winfo height $w] - (2 * [testmetrics cyvscroll $w])}]
 	} else {
-	    return [expr [winfo width $w] - 2*[testmetrics cxhscroll $w]]
+	    return [expr {[winfo width $w] - (2 * [testmetrics cxhscroll $w])}]
 	}	    
     } else {
-	if [string match v* [$w cget -orient]] {
-	    return [expr [winfo height $w] \
-		    - ([winfo width $w] \
+        if {[string match "v*" [$w cget -orient]]} {
+	    return [expr {[winfo height $w] \
+		    - ((([winfo width $w] \
 			- [$w cget -highlightthickness] \
-			- [$w cget -bd] + 1)*2]
+			  - [$w cget -borderwidth]) + 1) * 2)}]
 	} else {
-	    return [expr [winfo width $w] \
-		    - ([winfo height $w] \
+	    return [expr {[winfo width $w] \
+		    - ((([winfo height $w] \
 			- [$w cget -highlightthickness] \
-			- [$w cget -bd] + 1)*2]
+			  - [$w cget -borderwidth]) + 1) * 2)}]
 	}
     }
 }
@@ -43,8 +43,8 @@ proc getTroughSize {w} {
 # as you fix bugs and add features.
 
 foreach {width height} [wm minsize .] {
-    set height [expr ($height < 200) ? 200 : $height]
-    set width [expr ($width < 1) ? 1 : $width]
+    set height [expr {($height < 200) ? 200 : $height}]
+    set width [expr {($width < 1) ? 1 : $width}]
 } 
 
 frame .f -height $height -width $width
@@ -60,8 +60,8 @@ foreach test {
 	    {bad relief "non-existent": must be flat, groove, raised, ridge, solid, or sunken}}
     {-background #ff0000 #ff0000 non-existent
 	    {unknown color name "non-existent"}}
-    {-bd 4 4 badValue {bad screen distance "badValue"}}
-    {-bg #ff0000 #ff0000 non-existent
+    {-borderwidth 4 4 badValue {bad screen distance "badValue"}}
+    {-background #ff0000 #ff0000 non-existent
 	    {unknown color name "non-existent"}}
     {-borderwidth 1.3 1 badValue {bad screen distance "badValue"}}
     {-command "set x" {set x} {} {}}
@@ -115,14 +115,14 @@ test scrollbar-2.4 {Tk_ScrollbarCmd procedure} {
 	    [info command .s]
 } {1 {unknown option "-gorp"} 0 {}}
 test scrollbar-2.5 {Tk_ScrollbarCmd procedure} -setup {
-    catch {destroy .s}
+    destroy .s
 } -body {
     scrollbar .s
 } -cleanup {
     destroy .s
 } -result .s
 
-scrollbar .s -orient vertical -command scroll -highlightthickness 2 -bd 2
+scrollbar .s -orient vertical -command scroll -highlightthickness 2 -borderwidth 2
 pack .s -side right -fill y
 update
 test scrollbar-3.1 {ScrollbarWidgetCmd procedure} {
@@ -162,7 +162,7 @@ test scrollbar-3.10 {ScrollbarWidgetCmd procedure, "cget" option} {
 } {0 vertical}
 scrollbar .s2
 test scrollbar-3.11 {ScrollbarWidgetCmd procedure, "cget" option} {
-    expr {[.s2 cget -bd] == [lindex [.s2 configure -bd] 3]}
+    expr {[.s2 cget -borderwidth] == [lindex [.s2 configure -borderwidth] 3]}
 } 1
 test scrollbar-3.12 {ScrollbarWidgetCmd procedure, "cget" option} emptyTest {
     # empty test; duplicated scrollbar-3.11
@@ -218,10 +218,10 @@ test scrollbar-3.25 {ScrollbarWidgetCmd procedure, "delta" option} {
 } {0}
 test scrollbar-3.26 {ScrollbarWidgetCmd procedure, "delta" option} {
     format {%.6g} [.s delta 0 20]
-} [format %.6g [expr 20.0/([getTroughSize .s]-1)]]
+} [format %.6g [expr {20.0 / ([getTroughSize .s] - 1)}]]
 test scrollbar-3.27 {ScrollbarWidgetCmd procedure, "delta" option} {
     format {%.6g} [.s delta 0 -20]
-} [format %.6g [expr -20.0/([getTroughSize .s]-1)]]
+} [format %.6g [expr {-20.0 / ([getTroughSize .s] - 1)}]]
 test scrollbar-3.28 {ScrollbarWidgetCmd procedure, "delta" option} {
     toplevel .t -width 250 -height 100
     wm geom .t +0+0
@@ -253,13 +253,13 @@ test scrollbar-3.34 {ScrollbarWidgetCmd procedure, "fraction" option} {
 } {1}
 test scrollbar-3.35 {ScrollbarWidgetCmd procedure, "fraction" option} {
     format {%.6g} [.s fraction 4 21]
-} [format %.6g [expr (21.0 - ([winfo height .s] - [getTroughSize .s])/2.0) \
-       /([getTroughSize .s] - 1)]]
+} [format %.6g [expr {(21.0 - (([winfo height .s] - [getTroughSize .s]) / 2.0)) \
+		  / ([getTroughSize .s] - 1)}]]
 test scrollbar-3.36 {ScrollbarWidgetCmd procedure, "fraction" option} unix {
     format {%.6g} [.s fraction 4 179]
 } {1}
 test scrollbar-3.37 {ScrollbarWidgetCmd procedure, "fraction" option} {testmetrics} {
-    format {%.6g} [.s fraction 4 [expr 200 - [testmetrics cyvscroll .s]]]
+  format {%.6g} [.s fraction 4 [expr {200 - [testmetrics cyvscroll .s]}]]
 } {1}
 test scrollbar-3.38 {ScrollbarWidgetCmd procedure, "fraction" option} unix {
     format {%.6g} [.s fraction 4 178]
@@ -281,9 +281,9 @@ test scrollbar-3.41 {ScrollbarWidgetCmd procedure, "fraction" option} {
     format {%.6g} [.t.s fraction 100 0]
 } {0.5}
 if {[testConstraint testmetrics]} {
-    place configure .t.s -width [expr 2*[testmetrics cxhscroll .t.s]+1]
+    place configure .t.s -width [expr {(2 * [testmetrics cxhscroll .t.s]) + 1}]
 } else {
-    place configure .t.s -width [expr [winfo reqwidth .t.s] - 4]
+    place configure .t.s -width [expr {[winfo reqwidth .t.s] - 4}]
 }
 update
 test scrollbar-3.42 {ScrollbarWidgetCmd procedure, "fraction" option} {
@@ -408,25 +408,25 @@ test scrollbar-3.74 {ScrollbarWidgetCmd procedure} {
 } {1 {bad option "c": must be activate, cget, configure, delta, fraction, get, identify, or set}}
 
 test scrollbar-4.1 {ScrollbarEventProc procedure} {
-    catch {destroy .s1}
-    scrollbar .s1 -bg #543210
+    destroy .s1
+    scrollbar .s1 -background #543210
     rename .s1 .s2
     set x {}
     lappend x [winfo exists .s1]
-    lappend x [.s2 cget -bg]
+    lappend x [.s2 cget -background]
     destroy .s1
     lappend x [info command .s?] [winfo exists .s1] [winfo exists .s2]
 } {1 #543210 {} 0 0}
 
 test scrollbar-5.1 {ScrollbarCmdDeletedProc procedure} {
-    catch {destroy .s1}
+    destroy .s1
     scrollbar .s1
     rename .s1 {}
     list [info command .s?] [winfo exists .s1]
 } {{} 0}
 
-catch {destroy .s}
-scrollbar .s -orient vertical -relief sunken -bd 2 -highlightthickness 2
+destroy .s
+scrollbar .s -orient vertical -relief sunken -borderwidth 2 -highlightthickness 2
 pack .s -side left -fill y
 .s set .2 .4
 update
@@ -444,16 +444,16 @@ test scrollbar-6.6 {ScrollbarPosition procedure} unix {
     .s identify 19 100
 } {}
 test scrollbar-6.7 {ScrollbarPosition procedure} {
-    .s identify [expr [winfo width .s] / 2] -1
+    .s identify [expr {[winfo width .s] / 2}] -1
 } {}
 test scrollbar-6.8 {ScrollbarPosition procedure} {
-    .s identify [expr [winfo width .s] / 2] [expr [winfo height .s]]
+    .s identify [expr {[winfo width .s] / 2}] [expr {[winfo height .s]}]
 } {}
 test scrollbar-6.9 {ScrollbarPosition procedure} {
-    .s identify -1 [expr [winfo height .s] / 2]
+    .s identify -1 [expr {[winfo height .s] / 2}]
 } {}
 test scrollbar-6.10 {ScrollbarPosition procedure} {
-    .s identify [winfo width .s] [expr [winfo height .s] / 2]
+    .s identify [winfo width .s] [expr {[winfo height .s] / 2}]
 } {}
 test scrollbar-6.11 {ScrollbarPosition procedure} unix {
     .s identify 8 4
@@ -462,10 +462,10 @@ test scrollbar-6.12 {ScrollbarPosition procedure} unix {
     .s identify 8 19
 } {arrow1}
 test scrollbar-6.14 {ScrollbarPosition procedure} win {
-    .s identify [expr [winfo width .s] / 2] 0 
+    .s identify [expr {[winfo width .s] / 2}] 0 
 } {arrow1}
 test scrollbar-6.15 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] [expr [testmetrics cyvscroll .s] - 1]
+    .s identify [expr {[winfo width .s] / 2}] [expr {[testmetrics cyvscroll .s] - 1}]
 } {arrow1}
 test scrollbar-6.16 {ScrollbarPosition procedure} unix {
     .s identify 8 20
@@ -476,11 +476,11 @@ test scrollbar-6.17 {ScrollbarPosition procedure} {unix nonPortable} {
     .s identify 8 51
 } {trough1}
 test scrollbar-6.18 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] [testmetrics cyvscroll .s]
+    .s identify [expr {[winfo width .s] / 2}] [testmetrics cyvscroll .s]
 } {trough1}
 test scrollbar-6.19 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] [expr int(.2 / [.s delta 0 1]) \
-						+ [testmetrics cyvscroll .s] - 1]
+    .s identify [expr {[winfo width .s] / 2}] [expr  {int (.2 / [.s delta 0 1]) \
+						     + [testmetrics cyvscroll .s] - 1}]
 } {trough1}
 test scrollbar-6.20 {ScrollbarPosition procedure} unix {
     .s identify 8 52
@@ -491,12 +491,12 @@ test scrollbar-6.21 {ScrollbarPosition procedure} {unix nonPortable} {
     .s identify 8 83
 } {slider}
 test scrollbar-6.22 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] \
-	[expr int(.2 / [.s delta 0 1] + 0.5) + [testmetrics cyvscroll .s]]
+    .s identify [expr {[winfo width .s] / 2}] \
+      [expr { int (.2 / [.s delta 0 1] + 0.5) + [testmetrics cyvscroll .s]}]
 } {slider}
 test scrollbar-6.23 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] [expr int(.4 / [.s delta 0 1]) \
-						 + [testmetrics cyvscroll .s] - 1]
+  .s identify [expr {[winfo width .s] / 2}] [expr { int (.4 / [.s delta 0 1]) \
+						     + [testmetrics cyvscroll .s] - 1}]
 } {slider}
 test scrollbar-6.24 {ScrollbarPosition procedure} unix {
     .s identify 8 84
@@ -509,12 +509,12 @@ test scrollbar-6.27 {ScrollbarPosition procedure} {testmetrics win knownBug} {
     # bug in that GetSystemMetrics(SM_CYVTHUMB) actually returns a value
     # that is larger than the thumb displayed, skewing the ability to
     # calculate the trough2 area correctly (Win2k).  -- hobbs
-    .s identify [expr [winfo width .s] / 2] [expr int(.4 / [.s delta 0 1]) \
-						 + [testmetrics cyvscroll .s]]
+    .s identify [expr {[winfo width .s] / 2}] [expr { int (.4 / [.s delta 0 1]) \
+						      + [testmetrics cyvscroll .s]}]
 } {trough2}
 test scrollbar-6.28 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2] [expr [winfo height .s] \
-						 - [testmetrics cyvscroll .s] - 1]
+    .s identify [expr {[winfo width .s] / 2}] [expr {[winfo height .s] \
+						     - [testmetrics cyvscroll .s] - 1}]
 } {trough2}
 test scrollbar-6.29 {ScrollbarPosition procedure} unix {
     .s identify 8 180
@@ -523,11 +523,11 @@ test scrollbar-6.30 {ScrollbarPosition procedure} unix {
     .s identify 8 195
 } {arrow2}
 test scrollbar-6.32 {ScrollbarPosition procedure} {testmetrics win} {
-    .s identify [expr [winfo width .s] / 2]  [expr [winfo height .s] \
-						  - [testmetrics cyvscroll .s]]
+    .s identify [expr {[winfo width .s] / 2}]  [expr {[winfo height .s] \
+						      - [testmetrics cyvscroll .s]}]
 } {arrow2}
 test scrollbar-6.33 {ScrollbarPosition procedure} win {
-    .s identify [expr [winfo width .s] / 2] [expr [winfo height .s] - 1]
+    .s identify [expr {[winfo width .s] / 2}] [expr {[winfo height .s] - 1}]
 } {arrow2}
 test scrollbar-6.34 {ScrollbarPosition procedure} unix {
     .s identify 4 100
@@ -539,13 +539,13 @@ test scrollbar-6.37 {ScrollbarPosition procedure} win {
     .s identify 0 100
 } {trough2}
 test scrollbar-6.38 {ScrollbarPosition procedure} win {
-    .s identify [expr [winfo width .s] - 1] 100
+    .s identify [expr {[winfo width .s] - 1}] 100
 } {trough2}
 
-catch {destroy .t}
+destroy .t
 toplevel .t -width 250 -height 150
 wm geometry .t +0+0
-scrollbar .t.s -orient horizontal -relief sunken -bd 2 -highlightthickness 2
+scrollbar .t.s -orient horizontal -relief sunken -borderwidth 2 -highlightthickness 2
 place .t.s -width 200
 .t.s set .2 .4
 update
@@ -554,20 +554,20 @@ test scrollbar-6.39 {ScrollbarPosition procedure} unix {
     .t.s identify 4 8
 } {arrow1}
 test scrollbar-6.40 {ScrollbarPosition procedure} win {
-    .t.s identify 0 [expr [winfo height .t.s] / 2]
+    .t.s identify 0 [expr {[winfo height .t.s] / 2}]
 } {arrow1}
 test scrollbar-6.41 {ScrollbarPosition procedure} unix {
     .t.s identify 82 8
 } {slider}
 test scrollbar-6.43 {ScrollbarPosition procedure} {testmetrics win} {
-    .t.s identify [expr int(.4 / [.t.s delta 1 0]) + [testmetrics cxhscroll .t.s] \
-		       - 1] [expr [winfo height .t.s] / 2] 
+    .t.s identify [expr { int (.4 / [.t.s delta 1 0]) + [testmetrics cxhscroll .t.s] \
+			 - 1}] [expr {[winfo height .t.s] / 2}] 
 } {slider}
 test scrollbar-6.44 {ScrollbarPosition procedure} unix {
     .t.s identify 100 18
 } {trough2}
 test scrollbar-6.46 {ScrollbarPosition procedure} win {
-    .t.s identify 100 [expr [winfo height .t.s] - 1]
+    .t.s identify 100 [expr {[winfo height .t.s] - 1}]
 } {trough2}
 
 test scrollbar-7.1 {EventuallyRedraw} {
@@ -579,7 +579,7 @@ test scrollbar-7.1 {EventuallyRedraw} {
     lappend result [.s cget -orient]
 } {horizontal vertical}
 
-catch {destroy .t}
+destroy .t
 toplevel .t
 wm geometry .t +0+0
 test scrollbar-8.1 {TkScrollbarEventProc: recursive deletion} {
@@ -594,7 +594,7 @@ test scrollbar-8.1 {TkScrollbarEventProc: recursive deletion} {
     .t.f.s set 0 .5
     update
     set result [winfo exists .t.f.s]
-    event generate .t.f.s <ButtonPress> -button 1 -x [expr [winfo width .t.f.s] / 2] -y 5
+    event generate .t.f.s <ButtonPress> -button 1 -x [expr {[winfo width .t.f.s] / 2}] -y 5
     event generate .t <ButtonRelease> -button 1
     update
     lappend result [winfo exists .t.f.s] [winfo exists .t.f]
@@ -613,7 +613,7 @@ test scrollbar-8.2 {TkScrollbarEventProc: recursive deletion} {
     .t.f.s set 0 .5
     update
     set result [winfo exists .t.f.s]
-    event generate .t.f.s <ButtonPress> -button 1 -x [expr [winfo width .t.f.s] / 2] -y 5
+    event generate .t.f.s <ButtonPress> -button 1 -x [expr {[winfo width .t.f.s] / 2}] -y 5
     event generate .t.f <ButtonRelease> -button 1
     update
     lappend result [winfo exists .t.f.s] [winfo exists .t.f]
@@ -625,15 +625,14 @@ set l [interp hidden]
 deleteWindows
 
 test scrollbar-9.1 {scrollbar widget vs hidden commands} {
-    catch {destroy .s}
+    destroy .s
     scrollbar .s
     interp hide {} .s
     destroy .s
     list [winfo children .] [interp hidden]
 } [list {} $l]
 
-catch {destroy .s}
-catch {destroy .t}
+destroy .s .t
 
 # cleanup
 cleanupTests