ALWAYS use use strict; use warnings;!

use v5.14; has the side effect of doing use strict;, so that's good.

But use warnings; is missing. And using it would have caught some errors in your program.

The arguments being passed to fib make no sense.

You should originally pass the first and second terms (0 and 1), but you don't do that.

On the second pass, you should pass the second and third terms, but you don't do that either.

my $increment = @_; doesn't do what you think it does. An array in scalar context produces the number of elements in the array. You want

# `my ( ... ) = ...` evaluates the RHS in list context.
my ( $increment ) = @_;
[download]

# `shift` is short for `shift( @_ )`
my $increment = shift;
[download]

The program should output complete lines.

print $result;
[download]

print "$result\n";
[download]

say $result;
[download]

This is just a style thing, but

$result += $increment;
[download]

is cleaner than

$result = $result + $increment;
[download]

as Fletch pointed out.

Another style thing: 4000000 can also be written as 4_000_000.

Things that jump out at first glance:

• Version 5.14 is pretty long in the tooth (if you're actually using 5.14; see the documentation on feature for what's twiddled by a specific bundle tag)
• @frame[0] etc is wrong. You mean $frame[0] to reference a single element of an array (using @foo is only for referencing the entire array or a slice of values).
• Writing $result += $increment is more idiomatic
• You call count but don't do anything with the return value
• Which is probably for the best because without an explicit return I'd actually need to go experimentally figure out what the implicit return value would be
• And now having written that I see that you've got $result declared outside the scope of count so I'll just mention that my functional preference would be to have an explicit argument rather than relying on shared state.

Edit: Just to be clear these aren't "terrible" problems (well aside from @ vs $ will someday cause yourself much pain if you don't cut that off quick but that's a common beginner thing; it's along the same lines why @_ vs using shift was mentioned elsewhere)

You call count but don't do anything with the return value

It's printed at the end.

The last value assigned to the (lexically declared file) global $result is printed. Nothing is done with the (implicit) return value from the sub count where it's called.</nitpick>

But yes eventually it's used. I was writing these in order as I read along so I didn't notice that action-at-a-distance because I glossed over the declaration on first read. Personal habit/preference is for a functional style (or OOP-y setters/getters at the worst :) and void context just looks plain wrong.

The cake is a lie.
The cake is a lie.
The cake is a lie.

Overall, a good start. The main some problems that i see:

• You are developing for an ancient version of Perl. Since then, subs (=functions) can now be declared to directly take function arguments.
• You are using recursion. Unless you really know what you are doing (and set strict recursion limits), that's something that can backfire quite easily. (And this isn't a problem that requires recursion).
• You are using global variables when you don't have to. That makes code re-use much harder.

Here is a version i quickly "modernized" and "cavacized". I'm note sure if the result is correct, though, since i don't have the description in the book available (and i had only a few minutes during my afternoon break to write this).

#!/usr/bin/perl

# Latest perl version with the newest features and bugfixes
use v5.40;

# Do all kinds of automatic checking of the program code
use strict;
use warnings;

# Great debugging tool to quickly print a complex structure (array, ha
+sh, scalar, whatever) to the console
use Data::Dumper;

# Call the calculation main routine
my $max = 100; # 4_000_000;
my @baseframe = (1, 2, 3);
my $result = fib($max, \@baseframe); # Call with a REFERENCE to @basef
+rame, so subs can modify it

# Print the final result
print 'Final: ', $result, "\n";

# For debugging, print the final state of @baseframe
print 'Final @baseframe: ', Dumper(\@baseframe), "\n";

# And end scene...
exit(0);


# This does ONE round of the calculation, no deep recursion
sub fib_one_round($frame) {
    $frame->[0] = $frame->[0] + $frame->[1];
    $frame->[1] = $frame->[2];
    $frame->[3] = $frame->[2];

    if(($frame->[0] %2) != 0) {
        return $frame->[0];
    }

    return 0;
}

# This does the main loop. We are working on an array *reference*, so 
+fib_one_round can modify the array
sub fib($max, $frame = [1, 2, 3]) {
    my $result = 0;

    # Loop until the max value has been reached
    while($result < $max) {
        $result += fib_one_round($frame);

        # Print the intermediate results for debugging
        print $result, "\n";
    }

    return $result;
}
[download]

Basically, i replaced the recursion with a while loop, added modern sub-signatures, made sure the "frame" we are working on is passed through the function calls (instead of global vars). And removed/inlined the count() function because it's a single if-statement that doesn't warrant the performance overhead of a funtion call. And, oh, added an example of Data::Dumper to show how you can easily look at complex data structures for debugging.

I also moved the main code above the subs, because that makes it easier if you come back at a later time to the code. The high level stuff is at the beginning (showing/explaining what the script does), the nitty-gritty details that you might never have to touch again are below that.

Hope that helps.

You should have told us that it's Euler Problem #2

I took some liberties, especially using unshift and pop (which is kind of optional) facilitated to fix a conceptional bug of yours¹.

Feel free to ask if you have problems understanding.

HTH! :)

PS: @Harangzsolt33 I had no problems calculating for 4e12 or bigger, the limit is rather integer accuracy and not recursion overhead.

#!/usr/bin/perl
use v5.14;
use strict;
use warnings;

my $result  = 0;
my $level   = 2;        # next is F2
my $max_val = 4_000_000;
my $max_rec = 1000;

my $DBG = 1;


fib(1,0);            # F1,F0
print $result;


sub fib
{
    my  @frame = @_;

    #my $DBG = 1;

    unshift @frame, $frame[0] + $frame[1];   # prepend next level Fib 
+(see footnote 1)
    pop @frame;                              # remove tailing Fib (opt
+ional)

    count($frame[0]);

    say "Fib(" . $level++ . ") =  $frame[0]" if $DBG;
    say " ... # \@frame = (@frame)" if $DBG;

    # playing safe with deep recursion    
    die "May recursion $max_rec exceeded" if $level > $max_rec; 

    return fib(@frame) unless($frame[0] >= $max_val);
}

sub count {
    my ($increment) = @_;

    #my $DBG = 1;
    
    if(($increment  % 2) == 0){
    say "adding Fib($level) = $increment to $result" if $DBG;
    $result += $increment;
    }
}
[download]

activating the $DBG flags will show you debug information.

UPDATE

¹) you can also do a list assignment instead of unshift and pop. Your code suffered from timing problems when overwriting.

• @frame = ( $frame[0] + $frame[1], $frame[0] );

TIMTOWTDI

#!/usr/bin/perl

use strict; # https://perlmonks.org/?node_id=11165687
use warnings;

my ($result, $x, $y) = (0, 0, 1);
($result, $x, $y) = ($result + ($y & 1 ? 0 : $y), $y, $x + $y) while $
+y <= 4e6;
print "result $result\n";
[download]

Outputs:

result 4613732
[download]

#!/usr/bin/perl
use v5.14;
use strict;
use warnings;

my $result = 0;


sub fib
{
 my @frame = @_;

 $frame[0] += $frame[1];
 $frame[1] = $frame[2];
 $frame[2] = $frame[3];
 
 count($frame[0]);

 return fib(@frame) unless($frame[0] >= 4_000_000);
} 

sub count
{
        my $increment = shift;
        if(($increment  % 2) == 0)
        {
                $result += $increment;
        }
}

fib(1,2,3);
say $result;
[download]

You still pass 1, 2, 3 to the first call (instead of 0, 1)

More importantly, you still pass $frame[0]+$frame[1], $frame[2], $frame[3] to the subsequent calls . That's completely wrong. You need to pass the nth and (n+1)th terms.

So, like ikegami pointed out, always start your Perl programs with the following three lines:

#!/usr/bin/perl
use strict;
use warnings;

I also like to add:

$| = 1;

because it turns off buffering stdout, which means when you print something, it will immediately appear on the screen. Normally, when you print something, Perl will capture it and hold it in a buffer, and when the buffer gets full, then it will print it. So, you may get weird results where part of your program that you expect to produce some output appears to do nothing. So, basically, I always start my Perl programs with these four lines. The "use strict" and "use warnings" help you avoid weird problems in coding..

my $result = 0;

sub fib
{
 my  @frame = @_;

Note: When you are referring to a single element of an array or list, always put a $ sign in front of it instead of a @. When you use @ in front of an array or list, it means that you are referring to all of the elements. So, this code:

# @frame[0] = @frame[0] + @frame[1];
# @frame[1] = @frame[2];
# @frame[3] = @frame[2];

is better written as:

 $frame[0] += $frame[1];
 $frame[1] = $frame[2];
 $frame[3] = $frame[2];

You may notice that I also changed 

  @frame[0] = @frame[0] + @frame[1];

to

  $frame[0] += $frame[1];
  
just because it is shorter and nicer. 

 count($frame[0]);
[download]

Now, this is not a syntax error, but it's a program design error. Here in the next line you are calling the sub-routine itself, which can cause problems. If you call a sub-routine from within itself many times, that's often a bug. Why? Because each time you call a sub-routine, it uses memory. When you exit a sub, that memory is released. But if you call a sub-routine from within itself before exiting, then your program is just using more and more memory without the ability to release it, so this is a design flaw here. When you want to call a sub-routine from within itself, always think about the problem first and see if you can write the code in a different way, because this is leading to an undesired outcome. Here, in this case, your sub-routine is adding 2 to the first $frame[0] each time, and then the function calls itself millions of times. That means you are possibly using more than 10 megabytes of memory just to do this loop. So, try to redesign this code. I would create a for loop here instead.

 return fib(@frame) unless($frame[0] >= 4000000);
}

sub count
{
        my $increment = @_;
        if(($increment  % 2) == 0)
        {
                $result = $result + $increment;
        }

Another thing: When I do division by 2 and I'm looking
for the remainder, I always use & 1 instead of % 2 like so:

        if(($increment & 1) == 0)
[download]

Why? Because in the computer's internals, a division or multiplication is always slower than a logical operation. An & is a logical operation, because the processor can perform it in one step. A multiplication or division, on the other hand, can take hundreds of steps. So, you speed up your program if you avoid multiplication and divison whenever you can. In this case when you take a number and apply bitwise AND 1, it will leave you with the last digit, which is the same as dividing it by 2 and taking the remainder.

Or for example, when you are multiplying an integer by 2, I like to use the shift left operator << instead of multiplication. When I divide a number by 2, 4, 8, or 16... I use the shift right operator >> because it's faster. The processor can perform a shift in one step, while a multiplication can take longer, and division takes much longer. Example: $A << 3 is the equivalent of $A * 8

(Note: What I just told you about using bitwise AND and SHIFT only applies to integers under 4 billion. If you are making calculations with really big numbers that are far bigger than 4 billion, then you have to use the multiplication and division signs... ( * / % ). But for small numbers, you can use the bitwise operators ( & << >> ), because they are faster.)

}

fib(1,2,3);
print $result;

You're doing great!!

> If you call a sub-routine from within itself many times, that's often a bug.

Or it might be recursion, which is not a bug.

map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]

Please note that Perl has a default warning if the recursion level exceeds 100 (it's configurable somewhere)

:/tmp$ perl -wE'my $x=0; sub rec { return if ($x++ >= 99); rec()}; rec
+()'
Deep recursion on subroutine "main::rec" at -e line 1.                
+     # <---
:/tmp$ perl -wE'my $x=0; sub rec { return if ($x++ >= 98); rec()}; rec
+()'
[download]

It's true that every recursion can be written with a loop (and more importantly vice versa!).

But recursions provide very clean maintainable code for many problems and in my experience rarely exceed the 100 threshold, where the memory load is negligible.

FWIW Larry provided us with goto &sub syntax which allows to (re)call a sub without creating a frame and pushing data on the stack.

Most importantly:

Fibonacci is the schoolbook example for with recursive calls and you are WAYYYYYY OFF the mainstream here.

Even flatearthers might ridicule you as a weird outsider.

Cheers Rolf
_{(addicted to the Perl Programming Language :)
see Wikisyntax for the Monastery}

> Fibonacci is the schoolbook example for with recursive calls

Are you sure? I'm not sure what you mean exactly because of all the prepositions, but implementing the Fibonacci sequence by recursion is very ineffective inefficient. It's a schoolbook example of memoisation and implementing a recursive formula without recursion, I'd say.

I agree with the rest of your node, though.

Updated: Thanks LanX.

map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]

> (it's configurable somewhere)

Unfortunately it's not, one has to recompile Perl to change that. Mea culpa!

As a "funny" tangent, I asked ChatGPT and it hallucinated a special variable ${^RECURSION_LIMIT} ... LOL (sic)

Cheers Rolf
_{(addicted to the Perl Programming Language :)
see Wikisyntax for the Monastery}