comment on

Starting with your code:


my $y = 0;
my @list;

print "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\n
+";
print "<!DOCTYPE svg  PUBLIC \"-//W3C//DTD SVG 1.0//EN\"\n";
print "        \"http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.
+dtd\" >\n";
print "<svg height=\"200\" width=\"200\">\n";
print "  <line x1=\"0\" y1=\"0\" x2=\"80\" y2=\"0\"/>\n";
print "  <polyline\n";
print "    points=\"";
foreach (@scores) {
    if ($_) {
        push(@list,$y++);
        push(@list,$_ * -1.0);
    }
}
print join(",",@list);
print "\"\n";
print "    style=\"stroke: black; width: 1px; fill: none;\"/>\n";
print "</svg>\n";
[download]

My first inclination would be to clean up some of the escaped double quotes with heredocs:

my $y = 0;
my @list;

print <<PAGE_HEADER;
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE svg  PUBLIC "-//W3C//DTD SVG 1.0//EN"
 "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
PAGEHEADER

print <<SVG_HEADER;
 <svg height="200" width="200">
  <line x1="0" y1="0" x2="80" y2="0" />
SVG_HEADER

print "  <polyline\n";
print "    points=\"";

foreach (@scores) {
    if ($_) {
        push(@list,$y++);
        push(@list,$_ * -1.0);
    }
}
print join(",",@list);
print "\"\n";
print "    style=\"stroke: black; width: 1px; fill: none;\"/>\n";

print <<SVG_FOOTER;
</svg>
SVG_FOOTER
[download]

Next I would move the variable declarations down to where they are used -- otherwise it looks too much like C code. I put all of the print statements together since they are all working to priduce this one complicated line. I also explicitly test for non-zero-ness.

print <<PAGE_HEADER;
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE svg  PUBLIC "-//W3C//DTD SVG 1.0//EN"
 "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
PAGEHEADER

print <<SVG_HEADER;
 <svg height="200" width="200">
  <line x1="0" y1="0" x2="80" y2="0" />
SVG_HEADER

my $y = 0;
my @list;
foreach (@scores) {
    if ($_ != 0) {
        push(@list,$y++);
        push(@list,$_ * -1.0);
    }
}
print "  <polyline\n";
print "    points=\"";
print join(",",@list);
print "\"\n";
print "    style=\"stroke: black; width: 1px; fill: none;\"/>\n";

print <<SVG_FOOTER;
</svg>
SVG_FOOTER
[download]

Then I clean up the polyline piece and use another heredoc:

print <<PAGE_HEADER;
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE svg  PUBLIC "-//W3C//DTD SVG 1.0//EN"
 "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
PAGEHEADER

print <<SVG_HEADER;
 <svg height="200" width="200">
  <line x1="0" y1="0" x2="80" y2="0" />
SVG_HEADER

my $y = 0;
my @list;
foreach (@scores) {
    if ($_ != 0) {
        push(@list,$y++);
        push(@list,$_ * -1.0);
    }
}
my $list = join(",",@list);

print <<SVG_BODY;
  <polyline points="$list" 
   style="stroke: black; width: 1px; fill: none;"/>
SVG_BODY

print <<SVG_FOOTER;

</svg>
SVG_FOOTER
[download]

Now that I can see clearly, I can collapse the last two heredocs into one.

print <<PAGE_HEADER;
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<!DOCTYPE svg  PUBLIC "-//W3C//DTD SVG 1.0//EN"
 "http://www.w3.org/TR/2001/REC-SVG-20010904/DTD/svg10.dtd">
PAGEHEADER

print <<SVG_HEADER;
 <svg height="200" width="200">
  <line x1="0" y1="0" x2="80" y2="0" />
SVG_HEADER

my $y = 0;
my @list;
foreach (@scores) {
    if ($_ != 0) {
        push(@list,$y++);
        push(@list,$_ * -1.0);
    }
}
my $list = join(",",@list);

print <<SVG_BODY;
  <polyline points="$list" 
   style="stroke: black; width: 1px; fill: none;"/>
</svg>
SVG_BODY
[download]

That's it .. I haven't added any subroutines (perhaps more code would have suggested this would have been a good route to take). Now you can see what's going on without having to fight your way through a maze of escaped double quotes.

--t. alex
but my friends call me T.

Update: Jeepers, and I forgot to collapse the two push statements into one .. well, that's left as an exercise for the reader. :)

In reply to Re: Refactoring Redux... by talexb
in thread Refactoring Redux... by hsmyers

Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!

Titles consisting of a single word are discouraged, and in most cases are disallowed outright.

Read Where should I post X? if you're not absolutely sure you're posting in the right place.

Please read these before you post! —

Posts may use any of the Perl Monks Approved HTML tags:

a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr

You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)

	For:		Use:
	&		`&`
	<		`<`
	>		`>`
	[		`[`
	]		`]`

Link using PerlMonks shortcuts! What shortcuts can I use for linking?

See Writeup Formatting Tips and other pages linked from there for more info.