2
\$\begingroup\$

Okay, I'll try and keep this as short as I can. First, I need to keep various options available to the user:

if ($c_verb eq 'on')
    {
    if ($c_enabling) ##if need eq, add || '' at declaration.
        {$category_id = 'xcomp';}
    elsif ($c_subject)
        {$category_id = 'subj';}
    elsif ($c_object)
        {$category_id = 'obj';}
    elsif ($c_prep)
        {$category_id = 'prep';}
    else
        {$category_id = 'allverb';}
    ... 

After this, a loop through different forms of the search word (verb) is done. In the loop there is a chain of if-elsif with that checking which of the $category_id was chosen, and correctly acting on the corpus as a result.

Then a second "heading" (over-arching radio buttons), with subheadings is examined:

elsif ($c_adj eq 'on')
    {
    if ($c_modnoun)
        {$category_id = 'amod';}
    else
        {$category_id = 'alladj';}
    ...

Again same structure as before this elsif, but a different file is opened, and a there is no need to loop through different forms of the search word.

Finally, to check if no button was pressed:

else ##No buttons, like original search.
    {
    $category_id = "all";
      ...

And again a similar structure is used, but no need to loop through forms and a different print is used.

This is common to all of them (except different files are opened and some regex's are modified for different things):

local $/ = 'Parsing'; ##Instead of breaking at standard newline (avoid join and split)

    open my $parse_corpus, '<', "/Users/jon/Desktop/stanford-postagger-full-2011-04-20/rootparsedLife1.txt" or die $!;

    while (my $sentblock = <$parse_corpus>) 
        {
        chomp $sentblock;

        next unless ($sentblock =~ /\[sent. \d+ len. \d+\]: \[(.+)\]/);
        $sentencenumber++;
        $sentence = $1;
        $sentence =~ s/, / /g;
        if ($sentence =~ /\~\s([\d+F][\.I_][\d\w]+)\s/ ) 
            {
            $chapternumber = $1;
            $sentencenumber = 0; ##Reset sentence number with new chapter
            }

        next unless ($sentblock =~ /\b$search_key/i); ##Ensure the sentence contains the searchkey
        next unless ($sentblock =~ /\(JJ\w*\s+\b$search_key\w*[\)\s]+/i); ##THIS Line is different for each

        my ($arg1, $arg2, $goodmatch);
        my @lines = split ("\n",$sentblock); ##Split by a newline

        for (my $l=0; $l < @lines; $l++)
            {
            $goodmatch = 0;
                    if ($category_id eq "subj")
                    {
                    if (($lines[$l] =~ /subj\w*\(|agent\w*\(/) && ($lines[$l] =~ /\b$verbform\b/i))
                        {

                      #.............. -elsif for each $category_id.......

And due to all the above options, there is an if(all)-else(everything else) used for the large, in-depth sort and print section.

Question Is there a common way to restructure this, I don't want to put my 500 line code here, so if there is no general method, I'll just delete the question. Even applying given/when? (recommended by Perl::Critic)

Constraints I'm running into memory problems, so it can't be inefficient (are subroutines more efficient or less??)

THANKS for your time! Let me know if something is unclear.


EDIT

Here is the breakdown of the project:

I am creating a search engine. User inputs search word and then has options to choose from (as radio buttons on a form):

--Show results where search word is a verb
-------enabling
-------subject
-------object
...
--Show results where search word is adjective
-------a modified noun (amod)
--Else show any instance search word appears

Then, the script searches through a file of format:

Parsing [sent. 2 len. 10]: [Radially, symmetrical, animals, move, slowly, or,
not, at, all, .]
(ROOT(S(NP(ADJP (RB Radially) (JJ symmetrical))(NNS animals))(VP (VBP 
move)(ADVP (RB slowly)(CC or)(RB not))(ADVP (IN at) (DT all)))    (. 
.)))
nsubj(move-4, animals-3)
advmod(move-4, at-8)
pobj(at-8, all-9)

And takes the info necessary depending on the user's input choices. Example:

Search: 'move'

Click: 'verbs only'-->'subjects'

Output (from excerpt above):

1 match(es) in which the subject of move is animals :

Section 1_1: Radially symmetrical animals move slowly or not at all .

That sums it up. The data structure I used was AoA, where the inner array contains all the info for each match. Memory has continued to pop in and out. From malloc_errors to one instance where the browser crashed. I don't think the script is at fault, but I do need it to be as fast as possible for searches, and CGI seems to be the 'bottleneck' there.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

In order to provide meaningful help, I would need to know more about your application and what you are trying to accomplish. You are asking about if..else, but I suspect the real question/answer that you need is along the lines of: "what data structure and approach is appropriate?". There are a number of look-up table driven techniques that might be appropriate - huge hunks of this if..else code may just wind up disappearing. But I cannot give you a concrete example without knowing a bit more.

I am curious where things like $c_subject come from? A bit more about the overall flow of your app would help put things in context.

Can you show an example of what the input looks like? From your other question, perhaps it is sections like this:

Parsing [sent. 1 len. 31]:
        nsubj(85-7, Processes-3)
        nn(Transport-6, Membrane-5)
        prep_of(Processes-3, Transport-6)
        nsubj(examined-10, We-8)
        nsubjpass(used-17, it-15)
        xsubj(perform-19, it-15)
        conj_and(examined-10, used-17)
        xcomp(used-17, perform-19)
        dobj(perform-19, function-22)
        prep_of(binding-25, cell-28)
Parsing [sent.2 len. 50]
        more of these tag descriptions here? 

So what does your application do with this? Is the full text part of this same file, a different file or data structure?

> I'm running into memory problems .. The code will not be a problem. More likely is some data storage/structure issue or perhaps even a memory leak. BTW, what leads you to believe that you have a memory problem? What symptom do you see?

Oh, using a better indenting style would help the readability of your code. Move the { and } back to the previous level of indenting:

if (some condition)
{
   statement...
}
else
{
   some other statement
}

I like the above better, but this is also acceptible:

if (something){
    statement1
    statement2
}elsif (something else){
    statement3
    statement4
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the tips, I will edit my question with some more details. \$\endgroup\$
    – Jon
    Jul 5, 2011 at 4:25
1
\$\begingroup\$

You should probably use a hash to store the information.

my %data = (
  xcomp => $c_enabling,
  subj  => $c_subject,
  obj   => $c_object,
  prep  => $c_prep,
);

If you had done it that way, it would be simple to set the value, without a huge if elsif chain.

my $category_id = 'allverb'; # set default

for my $category ( qw'xcomp subj obj prep' ){
  if( $data{$category} ){
    $category_id = $category;
  }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.