librelist archives

« back to archive

Refactoring write_datails

Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-24 @ 02:43
I refactored the function that write details in CSV file to take the short
name of metrics from the Analizo Metrics.

This modifications stay in branch "_csv_output_".

Someone could review my code?

Thanks.

Re: [analizo] Refactoring write_datails

From:
Paulo Meirelles
Date:
2013-10-24 @ 09:17
2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>

> I refactored the function that write details in CSV file to take the short
> name of metrics from the Analizo Metrics.
>
> This modifications stay in branch "_csv_output_".
>
> Someone could review my code?
>

I've liked it, but there are some indentation 'problems':

https://github.com/Analizo-UnB/analizo/blob/0b3ba2f181f89641d92b564519f70a5b6d7b90e8/lib/Analizo/Batch/Output/CSV.pm.
May you fix them?

thanks a lot!
-- 
Paulo Meirelles
FGA-UnB (http://fga.unb.br)
CCSL-IME/USP (http://ccsl.ime.usp.br)

Re: [analizo] Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-24 @ 17:07
Ok. I'll fix them.


2013/10/24 Paulo Meirelles <paulo@softwarelivre.org>

> 2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>
>
>> I refactored the function that write details in CSV file to take the
>> short name of metrics from the Analizo Metrics.
>>
>> This modifications stay in branch "_csv_output_".
>>
>> Someone could review my code?
>>
>
> I've liked it, but there are some indentation 'problems':
> 
https://github.com/Analizo-UnB/analizo/blob/0b3ba2f181f89641d92b564519f70a5b6d7b90e8/lib/Analizo/Batch/Output/CSV.pm.
> May you fix them?
>
> thanks a lot!
> --
> Paulo Meirelles
> FGA-UnB (http://fga.unb.br)
> CCSL-IME/USP (http://ccsl.ime.usp.br)
>

Re: [analizo] Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-25 @ 01:57
We have the problem that the word "push" is ambiguous, "push" is a name of
a subroutine of CSV.pm and the name used to put things in one array. This
problem generate a warning.


2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>

> Ok. I'll fix them.
>
>
> 2013/10/24 Paulo Meirelles <paulo@softwarelivre.org>
>
>> 2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>
>>
>>> I refactored the function that write details in CSV file to take the
>>> short name of metrics from the Analizo Metrics.
>>>
>>> This modifications stay in branch "_csv_output_".
>>>
>>> Someone could review my code?
>>>
>>
>> I've liked it, but there are some indentation 'problems':
>> 
https://github.com/Analizo-UnB/analizo/blob/0b3ba2f181f89641d92b564519f70a5b6d7b90e8/lib/Analizo/Batch/Output/CSV.pm.
>> May you fix them?
>>
>> thanks a lot!
>> --
>> Paulo Meirelles
>> FGA-UnB (http://fga.unb.br)
>> CCSL-IME/USP (http://ccsl.ime.usp.br)
>>
>
>

Re: [analizo] Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-25 @ 19:26
This is the error when we runned metrics-batch:

Ambiguous call resolved as CORE::push(), qualify as such

This did not happen when we sended the last pull request. Is this some perl
update?


2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>

> We have the problem that the word "push" is ambiguous, "push" is a name of
> a subroutine of CSV.pm and the name used to put things in one array. This
> problem generate a warning.
>
>
> 2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>
>
>> Ok. I'll fix them.
>>
>>
>> 2013/10/24 Paulo Meirelles <paulo@softwarelivre.org>
>>
>>> 2013/10/24 Lucas Kanashiro <kanashiro.duarte@gmail.com>
>>>
>>>> I refactored the function that write details in CSV file to take the
>>>> short name of metrics from the Analizo Metrics.
>>>>
>>>> This modifications stay in branch "_csv_output_".
>>>>
>>>> Someone could review my code?
>>>>
>>>
>>> I've liked it, but there are some indentation 'problems':
>>> 
https://github.com/Analizo-UnB/analizo/blob/0b3ba2f181f89641d92b564519f70a5b6d7b90e8/lib/Analizo/Batch/Output/CSV.pm.
>>> May you fix them?
>>>
>>> thanks a lot!
>>> --
>>> Paulo Meirelles
>>> FGA-UnB (http://fga.unb.br)
>>> CCSL-IME/USP (http://ccsl.ime.usp.br)
>>>
>>
>>
>

Re: [analizo] Refactoring write_datails

From:
Antonio Terceiro
Date:
2013-10-25 @ 19:55
On Fri, Oct 25, 2013 at 05:26:05PM -0200, Lucas Kanashiro wrote:
> This is the error when we runned metrics-batch:
> 
> Ambiguous call resolved as CORE::push(), qualify as such

you probably missed $self->  before push(), or there is some other bug

> This did not happen when we sended the last pull request. Is this some perl
> update?

I can look at it if you send an actual diff against master +
instructions on how to reproduce the problem.

-- 
Antonio Terceiro <terceiro@softwarelivre.org>
http://softwarelivre.org/terceiro

Re: [analizo] Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-25 @ 20:33
could not decode message

Re: [analizo] Refactoring write_datails

From:
Antonio Terceiro
Date:
2013-10-25 @ 21:29
> diff --git a/lib/Analizo/Batch/Output/CSV.pm b/lib/Analizo/Batch/Output/CSV.pm
> index 8a2dc22..4007c1c 100644
> --- a/lib/Analizo/Batch/Output/CSV.pm
> +++ b/lib/Analizo/Batch/Output/CSV.pm
> @@ -48,62 +48,50 @@ sub _encode_value($) {
>    return &$encoder($value);
>  }
>  
> +sub _extract_short_names_of_metrics {
> +	my $metrics_instance = new Analizo::Metrics();
> +	my @short_names = ();
> +
> +	my %metrics_names = $metrics_instance->list_of_metrics();
> +
> +	@short_names = keys %metrics_names;
> +
> +	return @short_names;
> +}
> +
>  sub _write_details {
>    my ($self, $id, $details) = @_;
>    my @array_of_values = ();
>    my $files_name;
>  
> +	my @fields = $self->_extract_short_names_of_metrics();
> +
>    my $csv_filename = $id. "-details.csv";
>    open my $csv_handler, '>'.$csv_filename  || die "Cannot open 
".$id."-details.csv\n".$!;
> +	
> +	print $csv_handler "filename".",module";
> +
> +	foreach (@fields){
> +		print $csv_handler ",".$_;
> +	}
> +
> +	print $csv_handler "\n";
>  
> -  print $csv_handler "filename,".
> -         "module,".
> -         "acc,".
> -         "accm,".
> -         "amloc,".
> -         "anpm,".
> -         "cbo,".
> -         "dit,".
> -         "lcom4,".
> -         "loc,".
> -         "mmloc,".
> -         "noa,".
> -         "noc,".
> -         "nom,".
> -         "npm,".
> -         "npa,".
> -         "rfc,".
> -         "sc\n";
> -
> -  foreach (@$details)
> -  {
> -    if($_->{_filename}[1] eq "")
> -    {
> +  foreach (@$details){
> +    if($_->{_filename}[1] eq ""){

this line also gives me a warning. You want to invert the test:

if($_->{_filename}[1]){
  # filename exists
} else {
  # filename does not exist
}

>        $file_name = $_->{_filename}[0];
>      }
> -    else
> -    {
> +    else{
>        $file_name = $_->{_filename}[0]."\/".$_->{_filename}[1];
>      }
>  
> -    push @array_of_values,  "".$file_name.",".
> -          "".$_->{_module}.",".
> -          $_->{acc}.",".
> -          $_->{accm}.",".
> -          $_->{amloc}.",".
> -          $_->{anpm}.",".
> -          $_->{cbo}.",".
> -          $_->{dit}.",".
> -          $_->{lcom4}.",".
> -          $_->{loc}.",".
> -          $_->{mmloc}.",".
> -          $_->{noa}.",".
> -          $_->{noc}.",".
> -          $_->{nom}.",".
> -          $_->{npm}.",".
> -          $_->{npa}.",".
> -          $_->{rfc}.",".
> -          $_->{sc}."\n";
> +		push @array_of_values, $file_name.",".$_->{_module};

you want CORE::push here

> +
> +		foreach $field (@fields){
> +			push @array_of_values, ",".$_->{$field};

and here

> +		}
> +
> +		push @array_of_values, "\n";

and here

the problem is that in these cases saying only "push" is ambiguous
because this same module also defines a method called "push".

I don't know why the push call (to CORE::push, but also not explicit)
inside the push method does not trigger any warning.

-- 
Antonio Terceiro <terceiro@softwarelivre.org>
http://softwarelivre.org/terceiro

Re: [analizo] Refactoring write_datails

From:
Lucas Kanashiro
Date:
2013-10-27 @ 19:52
Thanks Terceiro.


2013/10/25 Antonio Terceiro <terceiro@softwarelivre.org>

> > diff --git a/lib/Analizo/Batch/Output/CSV.pm
> b/lib/Analizo/Batch/Output/CSV.pm
> > index 8a2dc22..4007c1c 100644
> > --- a/lib/Analizo/Batch/Output/CSV.pm
> > +++ b/lib/Analizo/Batch/Output/CSV.pm
> > @@ -48,62 +48,50 @@ sub _encode_value($) {
> >    return &$encoder($value);
> >  }
> >
> > +sub _extract_short_names_of_metrics {
> > +     my $metrics_instance = new Analizo::Metrics();
> > +     my @short_names = ();
> > +
> > +     my %metrics_names = $metrics_instance->list_of_metrics();
> > +
> > +     @short_names = keys %metrics_names;
> > +
> > +     return @short_names;
> > +}
> > +
> >  sub _write_details {
> >    my ($self, $id, $details) = @_;
> >    my @array_of_values = ();
> >    my $files_name;
> >
> > +     my @fields = $self->_extract_short_names_of_metrics();
> > +
> >    my $csv_filename = $id. "-details.csv";
> >    open my $csv_handler, '>'.$csv_filename  || die "Cannot open
> ".$id."-details.csv\n".$!;
> > +
> > +     print $csv_handler "filename".",module";
> > +
> > +     foreach (@fields){
> > +             print $csv_handler ",".$_;
> > +     }
> > +
> > +     print $csv_handler "\n";
> >
> > -  print $csv_handler "filename,".
> > -         "module,".
> > -         "acc,".
> > -         "accm,".
> > -         "amloc,".
> > -         "anpm,".
> > -         "cbo,".
> > -         "dit,".
> > -         "lcom4,".
> > -         "loc,".
> > -         "mmloc,".
> > -         "noa,".
> > -         "noc,".
> > -         "nom,".
> > -         "npm,".
> > -         "npa,".
> > -         "rfc,".
> > -         "sc\n";
> > -
> > -  foreach (@$details)
> > -  {
> > -    if($_->{_filename}[1] eq "")
> > -    {
> > +  foreach (@$details){
> > +    if($_->{_filename}[1] eq ""){
>
> this line also gives me a warning. You want to invert the test:
>
> if($_->{_filename}[1]){
>   # filename exists
> } else {
>   # filename does not exist
> }
>
> >        $file_name = $_->{_filename}[0];
> >      }
> > -    else
> > -    {
> > +    else{
> >        $file_name = $_->{_filename}[0]."\/".$_->{_filename}[1];
> >      }
> >
> > -    push @array_of_values,  "".$file_name.",".
> > -          "".$_->{_module}.",".
> > -          $_->{acc}.",".
> > -          $_->{accm}.",".
> > -          $_->{amloc}.",".
> > -          $_->{anpm}.",".
> > -          $_->{cbo}.",".
> > -          $_->{dit}.",".
> > -          $_->{lcom4}.",".
> > -          $_->{loc}.",".
> > -          $_->{mmloc}.",".
> > -          $_->{noa}.",".
> > -          $_->{noc}.",".
> > -          $_->{nom}.",".
> > -          $_->{npm}.",".
> > -          $_->{npa}.",".
> > -          $_->{rfc}.",".
> > -          $_->{sc}."\n";
> > +             push @array_of_values, $file_name.",".$_->{_module};
>
> you want CORE::push here
>
> > +
> > +             foreach $field (@fields){
> > +                     push @array_of_values, ",".$_->{$field};
>
> and here
>
> > +             }
> > +
> > +             push @array_of_values, "\n";
>
> and here
>
> the problem is that in these cases saying only "push" is ambiguous
> because this same module also defines a method called "push".
>
> I don't know why the push call (to CORE::push, but also not explicit)
> inside the push method does not trigger any warning.
>
> --
> Antonio Terceiro <terceiro@softwarelivre.org>
> http://softwarelivre.org/terceiro
>
>
>