Beefy Boxes and Bandwidth Generously Provided by pair Networks
Welcome to the Monastery
 
PerlMonks  

compound statement

by props (Hermit)
on Oct 07, 2007 at 08:24 UTC ( [id://643232]=perlquestion: print w/replies, xml ) Need Help??

props has asked for the wisdom of the Perl Monks concerning the following question:

The following compound statement produces the wrong output. I would like you to point me to the right direction since this is above my Perl capacity. 
sub Esoda_Statistika_button {
my @totalExoda = 0;
my $sum = 0;
my $clearedValue=0;
chdir "/home/props/delice/eksoda" or die "didn't make  to eksoda folde
+r\n";

#for every  txt file in the directory
foreach my $file (glob '*.txt') {                   


# open that file
        open IN, $file; 
# read through that file line by line
        while (<IN>) { 
#if matches a digit at the specific form
              if ($_ =~ m/(Exoda.*Total:)(.*\d)/) {
#pick that number and store it in $2
               $clearedValue =$2;
#add  all these matched values to @totalExoda
              push(@totalExoda,$clearedValue);
              } #end while
# sum up numbers of @totalExoda and  store it in $sum;
               foreach (@totalExoda){
                $sum+= $clearedValue;
                print "$sum\n";  
               } #end if
                }       #end foreach       
                
           } #end foreach
   } #end sub

[download]
# the above logic is wrong because of its mistaken result, can you solve this algorithm ?

Replies are listed 'Best First'.
Re: compound statement
by FunkyMonk (Chancellor) on Oct 07, 2007 at 08:57 UTC
    Your biggest problem is your indentation. If your indentation is correct, you don't need comments telling you where each loop ends. Indeed, because your indentation was a mess, you had many closing braces commented incorrectly. Have a look at the reformatted code below. I've taken out most of the comments, but left some of the incorrect ones in. 
    sub Esoda_Statistika_button {
    my @totalExoda = 0;
    my $sum = 0;
    my $clearedValue=0;
    chdir "/home/props/delice/eksoda" or die "didn't make  to eksoda f
+older\n";

    foreach my $file (glob '*.txt') {
        open IN, $file;
        while (<IN>) {
            if ($_ =~ m/(Exoda.*Total:)(.*\d)/) {
                $clearedValue =$2;
                push(@totalExoda,$clearedValue);
            } #end while
            foreach (@totalExoda){
                $sum+= $clearedValue;
                print "$sum\n";
            } #end if
        } #end foreach
    }
}

    [download]

    It should be clear to you now that your inner foreach loop is inside the while but it should follow it (you don't want to sum all the digits until you've read the entire file): 

    sub Esoda_Statistika_button {
    my @totalExoda = 0;
    my $sum = 0;
    my $clearedValue=0;
    chdir "/home/props/delice/eksoda" or die "didn't make  to eksoda f
+older\n";

    foreach my $file (glob '*.txt') {
        open IN, $file;
        while (<IN>) {
            if ($_ =~ m/(Exoda.*Total:)(.*\d)/) {
                $clearedValue =$2;
                push(@totalExoda,$clearedValue);
            }
        }
        foreach (@totalExoda){
            $sum+= $_;
        }
        print "$sum\n";
    }
}

    [download]

    update: Added another correction spoted by by jeanluca

[reply]
[d/l]
[select]
    A reply falls below the community's threshold of quality. You may see it by logging in.
Re: compound statement
by jeanluca (Deacon) on Oct 07, 2007 at 08:51 UTC
    It might help if you tell us what you get and what you expect
    Anyway, I get the impression that this part is going wrong
    foreach (@totalExoda){
  $sum+= $clearedValue;
  print "$sum\n";     
} #end if

    [download]
    I think you want 
    
foreach (@totalExoda){
  $sum += $_;
  print "$sum\n";     
} # end of for loop

    [download]
    However, maybe you also want to put this loop outside the first 'for-loop' because your adding values to $sum multiple times this way.
    Hope this helps

    Cheers
    LuCa
[reply]
[d/l]
[select]
Re: compound statement
by moritz (Cardinal) on Oct 07, 2007 at 09:00 UTC
    the above logic is wrong because of its mistaken result, can you solve this algorithm ?

    That's not very specific. What is the desired output? What is the actual output? And most importantly: how do your input files look like?

    But I think the Problem is near the end of your code, where the foreach loop uses $_ implicitly as the loop variable, but you add $clearedValue to $sum, i.e. you are producing $n*$clearedValue, where $n is the number of items in the array @totalExoda

    Perl 6 in German

[reply]
[d/l]
[select]
Re: compound statement
by quester (Vicar) on Oct 07, 2007 at 09:11 UTC
    There are a couple of different problems.

    First, what is the problem to be solved? Is the subroutine supposed to print running total after each file is processed? And should the printed value be the total for just that file or for all the files that have been processed? Should the subroutine return a value?

    Next, my @totalExoda = 0; isn't written correctly, it could be written as @totalExoda = (0); to initialize the array to contain a single zero, but what you probably meant was @totalExoda = (); to initialize it with zero elements.

    When you add all the elements of @totalExoda, you need to say either 

        foreach (@totalExoda) {
        $sum += $_;
    }

    [download]
    or, more readably, 
        foreach my $x (@totalExoda) {
        $sum += $x;
    }

    [download]
    However, unless there is some reason that you need the array @totalExoda, it would be better to just sum as you go, something like this. (I am assuming you want to return $sum rather that printing it once per file, and this code is not tested): 
    sub Esoda_Statistika_button {
    my $sum = 0;
    chdir "/home/props/delice/eksoda" or die "didn't make  to eksoda f
+older\n";
    foreach my $file (glob '*.txt') {                   
        open my $in, "<", $file; 
        while (<$in>) { 
            $sum += $1 if /Exoda.*Total:(.*\d)/;
        }
    }       
    return $sum;
}

    [download]
    Notice that I also changed the two-argument form of open to the three argument form. This is to avoid possible disaster. Consider what would happen if someone created a very badly named file in the eskoda directory, for example with the following command to the shell. 
         echo "whatever" > "/home/props/delice/eksoda/| rm *"

    [download]
    The two argument form of open (without a "<" in front of the filename) will merrily delete all of your files when you run your script!
[reply]
[d/l]
[select]
      The two argument form of open (without a "<" in front of the filename) will merrily delete all of your files when you run your script!
      No it won't. open, with two arguments, defaults to opening for reading.

      update: As ysth pointed out this quote was referring to a specific use of open. Thanks ysth, and my apologies to quester. Next time I'll read your nodes more thoroughly.

[reply]
        Read the example above that statement more closely.
[reply]
      I really appreciate your help on this this is a sample txt file with content: 
      Supplier1           1
Supplier2           2
Supplier3           3


Exoda   Total:6 euros

      [download]
      i need to get from every input file that Total:6 euros ( The number 6 in this case). The logic behind it is that i need to add my daily shop expenses in a file everyday. So whenever i want to check my monthly or yearly expenses to run this subroutine and get the total expenses. So i need one number but i do something wrong i get the following output: 
      0
0
0
0
0
0
0
0
0
6
6
12
12
18
18
24
24
30
30
36
36
42
42
48
48
54
54
60
66
66
72
78
78
84
90
90
96
102
102
108
114
114
120
126
126
132
138
138
144
150
150
156
162
162
168
174
274

      [download]
      there is a confusion somewhere
[reply]
[d/l]
[select]
        well this code works for me 
        sub Esoda_Statistika_button {
my @totalExoda = ();
my $sum = 0;
my $clearedValue=0;
chdir "/home/props/delice/eksoda" or die "didn't make  to eksoda folde
+r\n";

foreach my $file (glob '*.txt') {  
        open IN, $file; 
        while (<IN>) { 
              if ($_ =~ m/(Exoda.*Total:)(.*\d)/) {
               $clearedValue =$2;
               push(@totalExoda,$clearedValue);
              } #if
              }   #end while
        } #end outer foreach
        foreach (@totalExoda){
                  $sum+= $_;
        } #end inner foreach
        chomp $sum;
        print "$sum\n"; 
        } #end sub

        [download]
        this thing stresses my head :)
[reply]
[d/l]
      A reply falls below the community's threshold of quality. You may see it by logging in.
A reply falls below the community's threshold of quality. You may see it by logging in.
A reply falls below the community's threshold of quality. You may see it by logging in.

Back to Seekers of Perl Wisdom

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?

www.com | www.net | www.org
Node Status?
node history
Node Type: perlquestion [id://643232]
Approved by FunkyMonk
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? • Last hour • Other CB clients
Other Users?
Others cooling their heels in the Monastery: (2)
As of 2024-04-24 17:47 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found