Beefy Boxes and Bandwidth Generously Provided by pair Networks RobOMonk
Don't ask to ask, just ask
 
PerlMonks  

Optimizing variable passing (code, peer review)

by deprecated (Priest)
on May 11, 2001 at 06:09 UTC ( #79629=perlquestion: print w/ replies, xml ) Need Help??
deprecated has asked for the wisdom of the Perl Monks concerning the following question:

sub simple_properties { my %properties = %{ shift() }; my ($emissive, $diffuse, $shininess, $specular, $transp); foreach my $property (keys %properties) { if ($property eq "emissive") { $emissive = "emissiveColor @{$properties{ $property }} "; } elsif ($property eq "diffuse") { $diffuse = "diffuseColor @{$properties{ $property }} "; } elsif ($property eq "shininess") { $shininess = "shininess @{$properties{ $property }} "; } elsif ($property eq "specular") { $specular = "specularColor @{$properties{ $property }} "; } elsif ($property eq "transparency") { $transp = "transparency @{$properties{ $property }} "; } } return <<"EOR"; appearance Appearance { material Material { $emissive $diffuse $shininess $specular $transp } } EOR }
Some of you may recognize my VRML endeavours from my home node. The above code does in fact work. And it's even pretty clear. However, every once in a while a great saint will come along and give me some suggestions that remarkably clarify what I am doing. So here are some things i'd like to clear up:
  • I don't like the creation of %properties but I dont see a real good way to do it other than how its being done.
  • I also dislike the use of separate strings for interpolation of the arrayrefs. However, I see them as being necessary for understanding the return value. Of course, there is more than one way to do it. I'm hoping somebody can come up with a way that is both simple and clean.
Here's some more code.
sub directional_light { my ($amb_intensity, $color_r, $direction_r, $intensity, $on) = (@_); my @color = @{ $color_r }; my @direction = @{ $direction_r }; $on = etf( $on ); return <<"EOR"; DirectionalLight { ambientIntensity $amb_intensity color @color direction @direction intensity $intensity on $on } EOR }
This part kind of irritates me as well. The heredoc here is much clearer. What bothers me though is the creation of the arrays using sort of 'placeholder' values (which japhy understandably hates). The only way I can see to work around this is :
my (@array, @other_array) = (@{$_[0]}, @{$_[1]});
which I find rather ugly and not particularly clear.

Anyhow, I'm particularly exhausted after a long day of hacking at the forthcoming (hopefully) VRML.pm. I hope my fellow monks can shed some light on this and save me a little time in refactoring.

yours truly,
el deppo

--
Laziness, Impatience, Hubris, and Generosity.

Comment on Optimizing variable passing (code, peer review)
Select or Download Code
Re: Optimizing variable passing (code, peer review)
by Masem (Monsignor) on May 11, 2001 at 07:15 UTC
    You can probably simplify the top by doing the following:
    my %property_hash = ( emmisive => 'emmisiveColor', diffuse => 'diffuseColor', shineness => 'shineness', speculary => 'specularColor', transparency => 'transparency' }; my $prop_string; foreach my $key ( keys %property_hash ) { # Mark! $prop_string .= $key . ' ' . @{$properties{ $key }} . "\n" if $properties{ $key }; } return <<"EOR"; appearance Appearance { material Material { $prop_string } } EOR
    Note that if you need defaults, you could also define these with the same keys as above in %prop_def, and then the marked line becomes:
    $prop_string .= $key . ' ' . ( @{$properties{ $key }} || @{$prop_def{ $key }} ) . "\n"

    Update on your second part. Unfortunately, getting the values out of @_ gets messy fast. Some other possible options:

    Use a hash to pass values in. This allows not only for later expansion without breaking the routines, but also for default values on your end. However, it's hard to do any compile-time checking for right parameters. EG:

    sub code { my %hash = %{ shift() }; my $value = $hash{ 'expected' } || $const_def_hash{ 'expected' }; }

    Taking a cue from the example above, use defencing of shifts to get your variables without the extra variable. You can still do some compile-time checking, but again, if you need to update the code, you might break other users code.

    sub code { # scalar, array ref, hash ref my $data = shift; my @array = @{ shift() }; my %hash = %{ shift() }; }

    Since all you seem to be doing is taking arguements and using that to pump out a text file for VRML, without doing any major processing of the code, you *may* want to consider using somelike like Template Toolkit 2, along with the hash argument suggestion, to make it easier to keep the perl code and VRML generatation separate.

    Note that I can see a nice way to tree all your data, and use a single call to TT2 to get everything out in one go, but that's probably beyond the scope of the help you are requesting.

    Update 2 - fixed an if condition above, thanks chipmunk!


    Dr. Michael K. Neylon - mneylon-pm@masemware.com || "You've left the lens cap of your mind on again, Pinky" - The Brain
(dws)Re: Optimizing variable passing (code, peer review)
by dws (Chancellor) on May 11, 2001 at 10:29 UTC
    Have you considered making properties first-class Objects? This could lead to greatly simplified code. Consider
    package EmmisiveColor; sub new { my $pkg = shift; return bless [ @_ ], $pkg; } sub as_string { my $self = shift; "emmisiveColor @$self" }
    And so on for the other properties. Properties become easier to enumerate. You can do away with the switch entirely. Assuming you're doing so within the context of an object that holds properties, your snippet reduces to something like:
    sub simple_properties { my $self = shift; my @propstrings = map { $_->as_string() } $self->properties(); return "appearance Appearance {\n" . " material Material {\n" . " ", join("\n ", @propstrings), "\n" . " }\n" . "}\n"; }
Re: Optimizing variable passing (code, peer review)
by perlmonkey (Hermit) on May 11, 2001 at 11:26 UTC
    I agree with dws, OOP definatley seems like it would fit with your project. Plus you could do fun things like overload the stringify operator by adding
    use overload "\"\"" => \&as_string;
    to the EmmisiveColor class. Then your simple_properties could become:
    sub simple_properties { my $self = shift; return "appearance Appearance {\n" . " material Material {\n" . " ". join("\n ", $self->properties). "\n" . " }\n" . "}\n"; }
    Overloading a string operator might be quite handy for what it looks like you are doing, and I bet you would find other significant simplifications with an OOP approach. Here is a quick overloading/oop tutorial. (shameless self promotion :)

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://79629]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others pondering the Monastery: (6)
As of 2014-04-20 08:30 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    April first is:







    Results (485 votes), past polls