RFC: Creating unicursal stars

All started a week ago when I saw a unicursal hexagram and I started wondering the logic behind drawing a n-points unicursal star, so I gone thorough some wikipedia articles and finally created a method that can draw a unicursal star polygon of n points for any natural* number.

For those wondering, any non-convex polygon in star form could be considered a star polygon, but only those where the lines can be draw starting and ending at the same point, passing through all the other points exactly one time are unicursal star polygons

Although my code works like I wanted, I still don't like how it does it, so I would like to hear what the perlmonks have to say about it.

And here is the code:

#!/usr/bin/perl
use warnings;

die "need number of points\n" unless @ARGV;
$n = shift;
die "need valid number\n" if($n!~m/^\d+$/ || $n < 1);

use constant PI => 4*atan2(1,1);
$rad = 150; # radius of circunference, size of star


$alpha = 3*PI/2; # angle of starting point
$beta = 2*PI/$n; # angle variation
for(0..$n-1) # finding necessary points
{
    $x = $rad + $rad * cos($alpha);
    $y = $rad + $rad * sin($alpha);

    $pts{$_}{x} = $x;
    $pts{$_}{y} = $y;

    $alpha += $beta;
}


# svg header
print "<?xml version='1.0' standalone='no'?>
<!DOCTYPE svg PUBLIC '-//W3C//DTD SVG 1.1//EN' 
'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd'>

<svg width='100%' height='100%' version='1.1'
xmlns='http://www.w3.org/2000/svg'>\n\n";

$mod = calcMagic($n); # interval of points
print STDERR "mod: $mod\n";

$oldp = 0;
$p = 1;

while($p != 0)
{
    $mod = hexMagic($oldp) if($n == 6); # hexagrams are irregular
    $p = ($oldp + $mod) % $n; # find next point

    print STDERR "LINE: $oldp <-> $p\n";

    $x1 =  $pts{$oldp}{x};
    $y1 =  $pts{$oldp}{y};

    $x2 =  $pts{$p}{x};
    $y2 =  $pts{$p}{y};

    print "<line x1='$x1' y1='$y1' x2='$x2' y2='$y2' style='stroke:rgb
+(0,0,0);stroke-width:1'/>\n";

    $oldp = $p;
}

print "\n</svg>\n";

# calculate the interval
# based on number of points
sub calcMagic
{
    ($n) = @_;
    $m = -1;
    $mod = 1;

    for(1..$n)
    {
        $mod += $m;
        $m = nextMagic($m);
    }
    return $mod;
}

# have no idea
sub nextMagic
{
    ($oldm) = @_;

    return 3 if($oldm == -1);
    return -2 if($oldm == 3);
    return 2 if($oldm == -2);
    return -1 if($oldm == 2);
}

# special case,
# hexagram intervals
sub hexMagic
{
    ($p) = @_;
    return 2 if($p == 0);
    return 3 if($p == 2);
    return 4 if($p == 5);
    return 4 if($p == 3);
    return 3 if($p == 1);
    return 2 if($p == 4);
}
[download]

Comment on RFC: Creating unicursal stars
Select or Download Code

Re: RFC: Creating unicursal stars
by huxtonr (Initiate) on Nov 06, 2009 at 16:27 UTC

The main problem isn't finding $mod, it's getting it to work, and you've done that. Congratulations!

However, there are a couple of things you might find it useful to change.

Firstly, "use strict" at the top of your script. Always. Until you know enough to ignore that advice.

Secondly, try having a short main body of code calling a couple of top-level functions:

#!/usr/bin/perl
...
my @polygon = build_polygon($num_points, $radius);
print_svg_polygon(@polygon);
exit;

sub build_polygon {
...
[download]

It makes it easy to see what the script does at a glance, and lets you re-use bits more easily.

Thirdly, make sure you scope your variables. Use "my" to restrict them to their defining block. Otherwise, the $n in calcMagic for example is the same as the $n at the top of your script. If you changed it inside the sub it would change in the main script too.

sub calcMagic
{
    my ($n) = @_;
    my $m   = -1;
    my $mod = 1;

    for(1..$n)
    {
        $mod += $m;
        $m = nextMagic($m);
    }
    return $mod;
}
[download]

Finally, read up on Getopt::Long (http://search.cpan.org/~jv/Getopt-Long-2.38/lib/Getopt/Long.pm). That will let you call add named options to your script so you can do something like:

draw_star.pl --points=5 --radius=200

It's a well-known and well tested module, and has good documentation.

You could also replace the multiple "if" statements with a lookup into an array, but I'm not sure it would make the code any clearer really.

Hope that's helpful.

[reply]
[d/l]
[select]

Re^2: RFC: Creating unicursal stars

by Anonymous Monk on Nov 06, 2009 at 18:14 UTC

There is a really good template at (tye)Re: Stupid question

[reply]

Re^3: RFC: Creating unicursal stars

by Your Mother (Monsignor) on Nov 07, 2009 at 16:54 UTC

That template is from 2000 (we're three versions of Perl past that time), was probably mostly developed even earlier, and while tye's advice is generally to be preferred over mine, I would call camelCase bad—or at the most defensible least, uncommon—Perl style.

[reply]

Re^4: RFC: Creating unicursal stars (agism)

by tye (Archbishop) on Nov 09, 2009 at 20:35 UTC

Re^5: RFC: Creating unicursal stars (agism)

by ikegami (Saint) on Nov 09, 2009 at 21:05 UTC

Some notes below your chosen depth have not been shown here

Re^5: RFC: Creating unicursal stars (agism)

by Your Mother (Monsignor) on Nov 09, 2009 at 23:09 UTC

Some notes below your chosen depth have not been shown here

Re^4: RFC: Creating unicursal stars

by Anonymous Monk on Nov 09, 2009 at 22:15 UTC

Re^5: RFC: Creating unicursal stars