CGI/Perl Guide | Learning Center | Forums | Advertise | Login
Site Search: in

  Main Index MAIN
INDEX
Search Posts SEARCH
POSTS
Who's Online WHO'S
ONLINE
Log in LOG
IN

Home: Perl Programming Help: Intermediate:
Is there a better way to do this?

 



panofish
Novice


Sep 5, 2009, 10:05 PM

Post #1 of 14 (2687 views)
Is there a better way to do this? Can't Post

I am writing a simple subroutine that will take as input a hash that represents the variable_name and variable_content pairs that I want to substitute into an html template using the text::template module. The point is that the html may contain any number of scalar variables that need to be substituted. Previously, I had always done this logic using an embedded subroutine and global variables which worked just fine, but it wasn't self documenting. This is my first step at being more object-oriented in my approach to programming. I realize it is NOT really OOP, but one step at a time.

My question is ... Is there a better way to accomplish this logic. It is my understanding that use of the eval statement is frowned upon? Would true OOP logic apply?


Code
my $message = "hello"; 

&mysub ("message"=>"howdy");

exit;

sub mysub {

my %arg = @_;
my $message;

print "message = $message\n";

while (($key, $value) = each(%arg)) {
print "$key = $value\n";
eval "\$$key=$value";
}

print "message = $message\n";

# run text::template logic to make variable substitutions into html

}


Alan Lilly http://www.panofish.net


panofish
Novice


Sep 6, 2009, 10:22 AM

Post #2 of 14 (2675 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post

OK, I've made a small improvement in my code. I believe this will prevent malicious use of the eval instruction. In the previous code, a user could input "rm *" and cause havok. And now the same input is interpolated as a string only.


Code
&mysub ("message"=>"exec 'mkdir junk'"); 
&mysub ("message"=>"`mkdir junk`");
&mysub ("message"=>"howdy");

sub mysub {

my %arg = @_;

while (($key, $value) = each(%arg)) {
eval "\$$key=". qq("$value");
}

print "message = $message\n";

# run text::template logic to make variable substitutions into html

}


Alan Lilly http://www.panofish.net

(This post was edited by panofish on Sep 6, 2009, 10:45 AM)


FishMonger
Veteran / Moderator

Sep 6, 2009, 11:18 AM

Post #3 of 14 (2667 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post

Using eval is not frowned upon as long as it's used where and when it's appropriate.

However, the use of symbolic references, which is what you're using in the eval statement, is definitely frowned upon.

http://www.perl.com/doc/FMTEYEWTK/style/slide24.html

This reference is old, but the info still holds true.
http://perl.plover.com/varvarname3.html


panofish
Novice


Sep 6, 2009, 11:27 AM

Post #4 of 14 (2664 views)
Re: [FishMonger] Is there a better way to do this? [In reply to] Can't Post

Thanks FishMonger. I knew I had a problem with global variables (those links help to clarify that).
Is there a better way to write this code?

Alan Lilly http://www.panofish.net


FishMonger
Veteran / Moderator

Sep 6, 2009, 11:51 AM

Post #5 of 14 (2657 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post


Code
use strict; 
use warnings;

mysub( q(exec 'mkdir junk') );
mysub( q(`mkdir junk`) );
mysub( q(howdy) );

sub mysub {

my $message = shift;
print "message = $message\n";

# run text::template logic to make variable substitutions into html

}


However, I doubt that's what you really want.

Instead of asking "Is there a better way to write this code?", you should be asking "is there a better way to accomplish X outcome?". You haven't provided enough details or code for me to answer that question.

Can you show me how and where this code fits into your script and details on what it needs to accomplish?


panofish
Novice


Sep 6, 2009, 1:05 PM

Post #6 of 14 (2654 views)
Re: [FishMonger] Is there a better way to do this? [In reply to] Can't Post

Here is a more complete example of my code. While creating it... I discovered that the text::template module substitution variables must be declared 'our' instead of 'my' when using strict.

I have include the perl code and the referenced html source below. The printhtml subroutine is meant to be a generic routine where I can pass it any html and any number of substitution variables. Then printhtml can substitute and output the desired html. Obviously, this code is part of some cgi web logic.

This logic DOES work.
I think this is the best my code can get.
I CHALLENGE the experts to prove me wrong. :)


Code
#! /usr/bin/perl  

use strict;
use Text::Template;

my $html;
our $userid;
our $name;

&printhtml (html=>"page.htm", userid=>"12345678", name=>"Alan Lilly" );

sub printhtml {

my %arg = @_;
my $key;
my $value;

while (($key, $value) = each(%arg)) {
eval "\$$key=". qq("$value");
}

my $template = Text::Template->new(SOURCE => "$html", DELIMITERS => [ '[', ']' ]);
my $result = $template->fill_in();
print "Content-Type: text/html\n\n";
print $result;

return(1);

}

===============================================================
page.htm
===============================================================

<html>
<head>
<title>example</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>
<body>
<table border="0" cellpadding="3" cellspacing="3" style="border-collapse: collapse" bordercolor="#111111" id="AutoNumber1">
<tr>
<td align="right">userid</td>
<td>[$userid]</td>
</tr>
<tr>
<td align="right">name</td>
<td><input name="name" type="text" value="[$name]" size="40" maxlength="40">
</td>
</tr>
<tr>
<td align="right"></td>
<td><br> <input name="button2" type="submit" value="Update" onClick="JavaScript:form1.action.value='myaccount_submit';" ></td>
</tr>
</table>
<p>
</body>

</html>


Alan Lilly http://www.panofish.net


panofish
Novice


Sep 6, 2009, 6:54 PM

Post #7 of 14 (2641 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post

Never-mind. I guess I should have read the text::template docs more thoroughly. It appears that what I'm am ultimately trying to do is part of the text::template mechanisms. Thanks for the help.

Alan Lilly http://www.panofish.net


FishMonger
Veteran / Moderator

Sep 6, 2009, 7:58 PM

Post #8 of 14 (2637 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post


Quote
think this is the best my code can get.
I CHALLENGE the experts to prove me wrong. :)


LOL, that's funny!

A beginner should never make that type of challenge unless they're sure they have the best solution, which they NEVER have.

I've already pointed out one of the problems, which you've ignored and this is looking more like your homework assignment, so why should I point out your other mistakes?


(This post was edited by FishMonger on Sep 6, 2009, 8:06 PM)


panofish
Novice


Sep 6, 2009, 8:15 PM

Post #9 of 14 (2633 views)
Re: [FishMonger] Is there a better way to do this? [In reply to] Can't Post

Thanks FishMonger. Wink I was just trying to goad a quicker response. You are the only person that tried to help and I appreciate it. And no.. this is not a homework assignment.

Alan Lilly http://www.panofish.net


FishMonger
Veteran / Moderator

Sep 7, 2009, 8:08 AM

Post #10 of 14 (2622 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post

Ok, I'll cave in and give you a better way of doing what you want.

As you can see, this doesn't use global vars, doesn't use eval, and doesn't use symbolic references.


Code
#!/usr/bin/perl 

use strict;
use warnings;
use Text::Template;
use Scalar::Util qw(reftype);
use CGI::Carp qw(fatalsToBrowser);

my $tmpl_args = {
userid => 12345678,
name => 'Alan Lilly',
};

print_html('page.htm', $tmpl_args);

sub print_html {

print "Content-Type: text/html\n\n";

my ($tmpl, $args) = @_;

unless(reftype $args eq 'HASH') {
die "invalid argument passed to template\n";
}

my $template = Text::Template->new(SOURCE => $tmpl,
DELIMITERS => [ '[', ']' ])
or die "Couldn't construct template: $Text::Template::ERROR\n";

my $result = $template->fill_in(HASH => $args);
print $result;

return 1;
}



(This post was edited by FishMonger on Sep 7, 2009, 8:09 AM)


panofish
Novice


Sep 7, 2009, 11:21 AM

Post #11 of 14 (2617 views)
Re: [FishMonger] Is there a better way to do this? [In reply to] Can't Post

Holy Moly! That is impressively elegant. This piece of code has now become my Rosetta stone. I will study, memorize, and use it for many future projects. To be honest... in my last message, I had given up. I had moved on in my code and didn't really expect anymore help. Even though you obviously have a great deal of experience... it still took some time and thought to create this nice piece of code ... THANKS!

Now that I see it, it seems so simple. Crazy

Alan Lilly http://www.panofish.net


FishMonger
Veteran / Moderator

Sep 7, 2009, 11:54 AM

Post #12 of 14 (2615 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post


Quote
Even though you obviously have a great deal of experience... it still took some time and thought to create this nice piece of code


It took me about 10 minutes only because I hadn't previously used the Text::Template module so I needed to scan through a portion of its documentation. The rest of the time was spent deciding if I wanted to post the solution. Due to your prior comment, I almost didn't post it.

It still could use some improvements, especially regarding the die statements.


panofish
Novice


Sep 7, 2009, 1:34 PM

Post #13 of 14 (2611 views)
Re: [FishMonger] Is there a better way to do this? [In reply to] Can't Post

Thanks for the feedback on my previous comment. Like most comments in a forum, intent and body language are masked, which can lead to misunderstanding. I wasn't trying to antagonize or instigate. I will avoid goofy statements like that in the future.

I have years of experience in perl, but I'm self taught and I've learned in a bubble because I have no local peers. I wish to improve my perl skills, make friends and acquire mentors through this forum.

Thanks again FishMonger... you've got a friend in me.

Alan Lilly http://www.panofish.net


KevinR
Veteran


Sep 8, 2009, 10:52 AM

Post #14 of 14 (2570 views)
Re: [panofish] Is there a better way to do this? [In reply to] Can't Post


In Reply To
Thanks for the feedback on my previous comment. Like most comments in a forum, intent and body language are masked, which can lead to misunderstanding. I wasn't trying to antagonize or instigate. I will avoid goofy statements like that in the future.

I have years of experience in perl, but I'm self taught and I've learned in a bubble because I have no local peers. I wish to improve my perl skills, make friends and acquire mentors through this forum.

Thanks again FishMonger... you've got a friend in me.


With the internet there is no reason or need to learn in a bubble. While it would be hard to impossible to teach a person perl through a venue such as a forum, it is a great resource and supplement to your personal efforts. I am sure you already know there are tons of online resources for the reading that can teach you perl after a fashion, and many are better than what we often see as school or course work assignments which are obviously teaching poor perl.

Regards,
Kevin
-------------------------------------------------

 
 


Search for (options) Powered by Gossamer Forum v.1.2.0

Web Applications & Managed Hosting Powered by Gossamer Threads
Visit our Mailing List Archives