You replace:

  my $digest = $due_digest->{$upcoming->{'borrowernumber'}} ||= {};
  $digest->{email} ||= $from_address;
  $digest->{count}++;

With:

  $due_digest->{$upcoming->{borrowernumber}}{email} = $from_address;
  $due_digest->{$upcoming->{borrowernumber}}{count}++;

But why exactly? The first syntax is Perlish, with ||=, but not to the point to be unreadable. And the second syntax uses autovivification which isn't that readable either. And the second syntax uses $h->{}{} which isn't so good: $h->{}->{} should be preferred, imho.

(In reply to Frédéric Demians from comment #3)
> You replace:
> 
>   my $digest = $due_digest->{$upcoming->{'borrowernumber'}} ||= {};
>   $digest->{email} ||= $from_address;
>   $digest->{count}++;
> 
> With:
> 
>   $due_digest->{$upcoming->{borrowernumber}}{email} = $from_address;
>   $due_digest->{$upcoming->{borrowernumber}}{count}++;
> 
> But why exactly?

Why create variables when you don't need to? Are we so desperate for speed that much that we want to lose simple readability?


> The first syntax is Perlish, with ||=, but not to the point
> to be unreadable.

To those unfamiliar with Perl "pointers" (for a lack of a better word, since it is a NEW syntax to me), it is unreadable. Once you realize that $digest
"points" to what $due_digest->{$upcoming->{'borrowernumber'}} points to, then it does make sense.


> And the second syntax uses autovivification which isn't
> that readable either.

Actually, autovivification is far more readable and understandable than using a secondary pointer to fill in another data structure.


> And the second syntax uses $h->{}{} which isn't so
> good: $h->{}->{} should be preferred, imho.

I agree with Frédéric Demians on this, though only because it makes the change more apparently equal to me, not because it actually generates a better or different data structure.

(In reply to M. Tompsett from comment #4)
> > And the second syntax uses $h->{}{} which isn't so
> > good: $h->{}->{} should be preferred, imho.
> 
> I agree with Frédéric Demians on this, though only because it makes the
> change more apparently equal to me, not because it actually generates a
> better or different data structure.

I have always used this syntax (4 years), and I won't change now :)

(In reply to Jonathan Druart from comment #5)
> (In reply to M. Tompsett from comment #4)
> > > And the second syntax uses $h->{}{} which isn't so
> > > good: $h->{}->{} should be preferred, imho.
> > 
> > I agree with Frédéric Demians on this, though only because it makes the
> > change more apparently equal to me, not because it actually generates a
> > better or different data structure.
> 
> I have always used this syntax (4 years), and I won't change now :)

Although $a->{b}->{c} is more readable, it seems that both variations are both widespread in Koha.
Changing a habit can be a good thing (at least sometimes) ..

I would also prefer $a->{b}->{c}. I think it's more readable.

Maybe we should vote on this for coding guidelines.

(In reply to Kyle M Hall from comment #9)
> Maybe we should vote on this for coding guidelines.

I added it as an agenda item to
http://wiki.koha-community.org/wiki/Development_IRC_meeting_4_February_2015#Agenda

In the perl world using $x->{one}->{two} rather than $x->{one}{two} is the most common. one reason is that the arrow syntax can be used in a string
e.g. print "Var: $x->{one}->{two}"

using the non-arrow syntax in this way does not do what you may think it does

Plus when you get to multiple levels of indirection the arrow-less version gets more ambigious

(In reply to Colin Campbell from comment #11)
> In the perl world using $x->{one}->{two} rather than $x->{one}{two} is the
> most common. one reason is that the arrow syntax can be used in a string
> e.g. print "Var: $x->{one}->{two}"
> 
> using the non-arrow syntax in this way does not do what you may think it does

Are you sure?
I often use it, 

  use Modern::Perl;
  my $h = { foo => { bar => 'foobar' } };
  say "my string with $h->{foo}->{bar}";
  say "my string with $h->{foo}{bar}";

outputs:

  my string with foobar
  my string with foobar

(In reply to Jonathan Druart from comment #12)
[SNIP]
> I often use it, 
> 
>   use Modern::Perl;
>   my $h = { foo => { bar => 'foobar' } };
>   say "my string with $h->{foo}->{bar}";
>   say "my string with $h->{foo}{bar}";
> 
> outputs:
> 
>   my string with foobar
>   my string with foobar

I wrote my own code to see the same thing. Which is why I said, "I agree with Frédéric Demians on this, though only because it makes the change more apparently equal to me, not because it actually generates a better or different data structure." back in comment #4. Structurally, it seems identical, but syntax-wise, I still prefer the $h->{foo}->{bar}.

Created attachment 35705 [details] [review]
[PASSED QA] Bug 13240: Remove some code obfuscation

my $bar;
my $foo = $bar->{borrowernumber} ||= {};
$foo->{one} ||= 'something';
$foo->{two}++;

What does $bar contain?

$VAR1 = {
          'borrowernumber' => {
                                'two' => 1,
                                'bar' => 'something'
                              }
        };

Not really obvious.

Maybe something I did not see is hidden.

Test plan:
Verify the digest for DUE and PREDUE work as before.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 35706 [details] [review]
[PASSED QA] Bug 13240: Remove commented warns

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 35707 [details] [review]
[PASSED QA] Bug 13240 [QA Followup]

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

(In reply to Jonathan Druart from comment #12)
> (In reply to Colin Campbell from comment #11)
> > In the perl world using $x->{one}->{two} rather than $x->{one}{two} is the
> > most common. one reason is that the arrow syntax can be used in a string
> > e.g. print "Var: $x->{one}->{two}"
> > 
> > using the non-arrow syntax in this way does not do what you may think it does
> 
> Are you sure?
> I often use it, 
> 
>   use Modern::Perl;
>   my $h = { foo => { bar => 'foobar' } };
>   say "my string with $h->{foo}->{bar}";
>   say "my string with $h->{foo}{bar}";
> 
> outputs:
> 
>   my string with foobar
>   my string with foobar

Without pausing fpr thought its not obvious which of the first three is doing what you want:
#!/usr/bin/perl
use feature qw( say );
my $list = [ 'foo', 'bar' ];
say "my string with $list[0]";
say "my string with ${list}[0]";
say "my string with ${$list}[0]";

say "my string with $list->[0]";

outputs:
my string with 
my string with ARRAY(0x936cb8)[0]
my string with foo
my string with foo

Patches pushed to master.

Thanks Jonathan!