--- loncom/homework/radiobuttonresponse.pm	2012/02/04 20:40:07	1.153.6.9
+++ loncom/homework/radiobuttonresponse.pm	2012/02/05 16:11:57	1.153.6.10
@@ -1,7 +1,7 @@
 # The LearningOnline Network with CAPA
 # mutliple choice style responses
 #
-# $Id: radiobuttonresponse.pm,v 1.153.6.9 2012/02/04 20:40:07 foxr Exp $
+# $Id: radiobuttonresponse.pm,v 1.153.6.10 2012/02/05 16:11:57 foxr Exp $
 #
 # Copyright Michigan State University Board of Trustees
 #
@@ -43,6 +43,10 @@ BEGIN {
         ('radiobuttonresponse') );
 }
 
+#---------------------------------------------------------------------------
+#
+#  Generic utility subs.
+
 sub bubble_line_count {
     my ( $numfoils, $bubbles_per_line ) = @_;
     my $bubble_lines;
@@ -54,6 +58,10 @@ sub bubble_line_count {
 
 }
 
+
+#------------------------------------------------------------------------------
+#
+#  XML handlers.
 sub start_radiobuttonresponse {
     my ( $target, $token, $tagstack, $parstack, $parser, $safeeval, $style ) =
       @_;
@@ -488,25 +496,25 @@ sub display_survey_html {
     #       foil lives in a <p>
     #
 
-    my $closing_html;
-    my $pre_foil;
-    my $post_foil;
-	
-    if ($direction eq 'horizontal') {
-	$result       .= '<table><tr>';
-	$closing_html = '</tr></table>';
-	$pre_foil     = '<td>';
-	$post_foil    = '</td>';
-    } else {
-	$pre_foil     = '<br />';
-    }
-    # Different rendering depending on whether answers are shown:
 
+    my ($opening_html, $closing_html, $pre_foil, $post_foil) = 
+	&html_direction_fragments($direction);
 
-    if ($showanswer) {
-	foreach my $name (@{$names}) {
+    $result = $opening_html;
+
+    # Different rendering depending on whether answers are shown:
+    # I played with different factorings but this seems the most concise/clear...
+    # although I don't like the $showanswer conditino inside the loop.  Other things I tried
+    #  - two loops..much longer code..no gain in clarity.
+    #  - Using a visitor patttern passing it the rendering code chunklets and
+    #    an anonymous hash reference for state data etc. Very cool but
+    #    quite a bit more code and quite a bit less clear.
+    
+    my $temp = 0;
+    foreach my $name (@{$names}) {
+	$result .= $pre_foil;
 
-	    $result .= $pre_foil;
+	if ($showanswer) {
 	    my $foiltext =  $Apache::response::foilgroup{$name . '.text'};
 
 	    # Bold the prior  response:
@@ -516,24 +524,17 @@ sub display_survey_html {
 	    } else {
 		$result .= $foiltext;
 	    }
-
-	    $result .= $post_foil;
-	}
-    } else {
-	my $temp = 0;
-	foreach my $name (@{$names}) {
-	    $result .=  $pre_foil;
-
+	} else {
 	    $result .= &html_radiobutton(
 		$part, $Apache::inputtags::response['-1'], $name, $lastresponse, $temp
 	     );
-
-	    $result .= $post_foil;
-	    $temp++;
-				       
 	}
+
+	$result .= $post_foil;
+	$temp++;
     }
 
+
     $result .= $closing_html;
     return $result;
 
@@ -631,7 +632,27 @@ sub latex_vertical_environment {
     }
 }
 
-
+##
+# Figure out the key html fragments that depend on the rendering direction:
+#
+# @param $direction - 'horizontal' for horizontal direction.
+#
+# @return list
+# @retval (part_start, part_end, foil_start, foil_end)
+# Where:
+#   - part_start is the HTML to emit at the start of the part.
+#   - part_end   is the HTML to emit at the end of the part.
+#   - foil_start is the HTML to emit prior to each foil.
+#   - foil_end is the HTML to emit after each foil
+#
+sub html_direction_fragments {
+    my $direction = shift;
+    if ($direction eq 'horizontal') {
+	return ('<table><tr>', '</tr></table>', '<td>', '</td>');
+    } else {
+	return ('', '<br />', '<br />', '');
+    }
+}
 
 ##
 #
@@ -1005,23 +1026,17 @@ sub display_foils_html {
     my ($whichfoils, $target, $direction, $part, $show_answer) = @_;
     my $result;
 
+
     # if the answers get shown, we need to label each item as correct or
     # incorrect.
 
-    if ($show_answer) {
-	my $item_pretext     = '<br />'; # html prior to each item
-	my $item_posttext    = '';	 # html after each item.
-	my $finalclose       = '';	 # html to close off the whole shebang
+    my ($opening_html, $finalclose, $item_pretext, $item_posttext) = 
+	&html_direction_fragments($direction);
 
+    $result .= $opening_html;
 
-	# Horizontal layout is a table with each foil in a cell
 
-	if ($direction eq 'horizontal') {
-	    $result        = '<table><tr>';
-	    $item_pretext  = '<td>' . $item_pretext;
-	    $item_posttext = '</td>';
-	    $finalclose    = '</tr></table>';
-	} 
+    if ($show_answer) {
 
 	foreach my $name (@{$whichfoils}) {
 
@@ -1055,25 +1070,10 @@ sub display_foils_html {
 	    $result .= "\n";	# make the html a bit more readable.
 	}
 
-	$result .= $finalclose;
 
     } else {
-	$result .= '<br />';	# end line prior to foilgroup:
-
-	#  Not showing the answers, we need to generate the HTML appropriate
-	#  to allowing the student to respond.
-	
-	my $item_pretext;
-	my $item_posttext;
 	my $lastresponse = &get_last_response($part);
-
-	if ( $direction eq 'horizontal' ) {
-	    $item_pretext  = '<td>';
-	    $item_posttext = '</td>';
-	}
-	else {
-	    $item_pretext = '<br/>';
-	}
+	
 	my $item_no = 0;
 	foreach my $name (@{$whichfoils}) {
 	    $result .= $item_pretext;
@@ -1084,13 +1084,9 @@ sub display_foils_html {
 	    $result .= $item_posttext;
 	    $item_no++;
 	}
-
-	if ($direction eq 'horizontal' ) {
-            $result .= "</tr></table>";
-        } else {
-	     $result .= "<br />"; 
-	}	
+	
     }
+    $result .= $finalclose;
 
     return $result;
 }
@@ -1114,7 +1110,7 @@ sub display_latex_exam {
     my $line          = 0;
     my $i             = 0;
 
-
+    
     if ($direction eq  'horizontal') {
 
 	# Marshall the display text for each foil and turn things over to
@@ -1124,7 +1120,6 @@ sub display_latex_exam {
 	$result .= &Apache::caparesponse::make_horizontal_latex_bubbles(
 	    $whichfoils, \@foil_texts, '$\bigcirc$');
 
-
     } else {
 	$result .= "\\begin{$venv}";