Hi, Shlomi.

Of course, you need to use strict and warnings. I'm sorry to not specify it
my answer.
And I'm agree with your suggestion about checking result of regexp
validation.

"For" cycle create very concrete and understandable lexical scope and you
can safely use loop variable outside this cycle.

You, of course, responsible not to write things like

for (@array) {
 say "original loop variable $_";
 foo($_);
 say "reversed loop variable $_";
}
sub foo {
  $_[0] = reverse $_[0];
}

but i think, it's important for beginners to understand such things like
scopes and modificatons value by reference and so on.

Of cource, when You  write nested cycles, You must specify loop variable
and label for next. But it's too redundant with simple loop.

About your regexp comments
1. Strongly agree and sorry for not doing it in this example
2. I don't consider it as a problem, maybe i'm wrong
3. It depends of what data You expects. Maybe good aprocach is to fetch
$earfcn as I did (\d+) and then check it length. Maybe it's better to
divide sting without regexp with split
($earfcn, $pcid) = split '-';

It depends how trustful an original source, do you need maintain your code
or it's one time job. And maybe some of this questions are beyond this
mailing list topic ))

Thank for your reply and thank for your code review

ср, 6 мая 2015 г. в 12:02, Shlomi Fish <shlo...@shlomifish.org>:

Hi Илья,
>
> some comments on your code:
>
> On Wed, 06 May 2015 08:09:01 +0000
> Илья Рассадин <elcaml...@gmail.com> wrote:
>
> > HI, Anirban.
> >
> > Regexp and hash of arrays can help you.
> >
> > my @a =
> >
> ('1900-0','1900-1','NULL','NULL','1900-2','1900-4','1902-5','1902-6','1902-7','1902-8');
> >
>
> Always start with "use strict;" and "use warnings;" or equivalent:
>
> http://perl-begin.org/tutorials/bad-elements/#no-strict-and-warnings
>
> > my %result;
> > foreach (@a) {
>
> It's not a good idea to use $_ as a loop variable because it can get
> clobberred/devastated too easily. Better use a "my" lexical variable to
> iterate:
>
> http://perl-begin.org/tutorials/bad-elements/#overuse_dollar_underscore
>
> >         next if $_ eq 'NULL';
>
> next should use an explicit loop label:
>
> http://perl-begin.org/tutorials/bad-elements/#flow-stmts-without-labels
>
> >         my ($earfcn, $pcid) = /^(\d+)-(.+)$/;
>
> 1. Always check that the regex match was succesful:
>
>
> http://perl-begin.org/tutorials/bad-elements/#regexes_wo_check_for_match_success
>
> 2. \d will also match Unicode digits . Better write it as [0-9].
>
> 3. You may wish to be more specific than ".+".
>
> >         push @{$result{$earfcn}}, $pcid;
> > }
> >
> > say "EARFCN=$_,PCID=" . join ("&", @{$result{$_}}) . ";" for sort keys
> > %result;
> >
>
> Again, an implicit loop using $_.
>
> Regards,
>
>         Shlomi Fish
>
> > Outcome is exatly what you want
> > EARFCN=1900,PCID=0&1&2&4;
> > EARFCN=1902,PCID=5&6&7&8;
> >
> >
>
> --
> -----------------------------------------------------------------
> Shlomi Fish       http://www.shlomifish.org/
> http://www.shlomifish.org/humour/Summerschool-at-the-NSA/
>
> Mastering ‘cat’ is almost as difficult as herding cats.
>     — http://www.shlomifish.org/humour/bits/Mastering-Cat/
>
> Please reply to list if it's a mailing list post - http://shlom.in/reply .
>
> --
> To unsubscribe, e-mail: beginners-unsubscr...@perl.org
> For additional commands, e-mail: beginners-h...@perl.org
> http://learn.perl.org/
>
>
>

Reply via email to