Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling

Re: Object Identifier? (red flags, more subs)

by Anonymous Monk
on Oct 22, 2018 at 00:41 UTC ( #1224432=note: print w/replies, xml ) Need Help??

in reply to Object Identifier?

I tried to add a parameter to the following code and it does not work.

Thats because that code is unorganized and full of "Red flags".

Organize your code, box up everything into subroutines (Every loop, every heredoc template, every giant if/else block).

This is a "Red flag" localtime ... $year + 1900, . Time::Piece and DateTime is what you want to use for date stamps/math, none of this +1900 stuff.

You would benefit heavily from reading "Red flag" as it was written exactly for you. Almost every example is one you have posted here.

Subroutines should take arguments not work on globals. You shouldn't mix who prints from where....

This is something like what your program should end up looking like

Main( @ARGV ); exit( 0 ); sub Main { my $pms = empire->new; my $query = CGI->new; my( $header, $body ) = Dispatch( $pms, $query ); print $header, $body; } sub Dispatch { my( $pms, $q ) = @_; if( $pms->authenticate( $q ) ){ my $page = $q->page || 'default'; return ThisPage( $pms, $q ) if $page eq 'thisone'; return ThatPage( $pms, $q ) if $page eq 'thatone'; return DefaultPage( $pms, $q ); } else { return UnauthorizedPage( $pqms, $q ); } } sub UnauthorizedPage { my( $pms, $q ) = @_; return RedirectTo($q, 'notloggedin.cgi'); } sub RedirectTo { my( $q , $page ) = @_; my $header = $q->redirect( UrlFor( $page ) ); my $body = ''; return $header, $body; } sub UrlFor { my( $name ) = @_; return $name; } sub DefaultPage { my( $pms, $q ) = @_; my $members = $pms->list_members; my $schedule = $pms->campaign_schedule; return $q->header, CampaignMembersHtml( $pms, $q, $members, $sched +ule ); } sub CampaignMembersHtml { my( $pms, $q, $members, $schedule ) = @_; my $html = MembersHtml( $q, $members ); $html .= ScheduleHtml( $q, $schedule ); return $html; } sub MembersHtml { ...; } sub ScheduleHtml { my( $q, $schedule ) =@_; my ( $status, $schedule_date, $max_emails, $clast60, $aolflag, $openflag, $yaho­o_flag, $shour, $smin, $old_mid, $old_dname, $cname, ) = @$schedule; ...; return $html; } sub Empire::authenticate { my( $pms, $query ) = @_; my $user = $query->param('user'); my $pass = $query->param('pass'); return !! $pms->check_security( $user, $pass ) } sub Empire::list_members { my( $pms, $q ) = @_; ... my $members = $dbh->selectall_arrayref($sql); ... return $members; } sub Empire::campaign_schedule { my( $pms, $q ) = @_; ... my $schedule = $dbh->selectall_arrayref($sql); ... return $schedule; }

Replies are listed 'Best First'.
Re^2: Object Identifier? (red flags, more subs)
by damfer21 (Novice) on Oct 22, 2018 at 12:26 UTC

    Thanks for the reply and help. I didn't write the code. I only added in $last240_flag. Every other last_flag works as intended.

    I don't know the first thing about Perl, just enough about general code to get myself in trouble. I posted the complete code in the reply above. If you have any other recommendations, I would appreciate the help.

      Hi damfer21,

      On first sight I don't see anything wrong with your modification. I just wonder if you also changed the file camp_copy_schedule_save.cgi? I mean, that is where the form action leads to. So I suspect you need to modify that file as well.


        Hi Veltro- I will check that out. Thank you so much. I am really new at this. I am curious about how the script knows that $last120_flag means to include data from the last 120 days. Should there be a file that contains those definitions or are they somehow predefined in the actual script?

Log In?

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1224432]
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others about the Monastery: (2)
As of 2022-08-11 05:33 GMT
Find Nodes?
    Voting Booth?

    No recent polls found