module bad design?)

[Olivier Laurent]

Hi there,

I'm playing with perl to understand how I could make modules.
Here is my "first module" ...And...It doesn't work.

This module should parse a CSV file an extract the appropriate columns
(it should return an array).

I call it with:
my $test= parse_file->new(FILE=>"./log/1fr.txt",AREA=>6);
my $arrayref=$test->getdata();

But it fails. I receive an error "could not open SCALAR(XXXX)";
it seems that FILE is transformed into a hexadecimal code ?
What is my mistake?


package parse_file;

use 5.001;

use strict;

# use warnings;


sub new {
my $self = {};
$self->{'FILE'} = undef;
$self->{AREA} = undef;
bless {$self};
}

sub getdata
{
my $self;
my $file = $self->{FILE};
my $area = $self->{AREA};
my @data;
my @return_data;
open(MYFILE,'$file')|| die "cannot open $file: $!";
while(defined($a=<MYFILE>))
{
my @data=split(/\,/,$a);
push(@return_data,$data[$area]);
}
close(MYFILE);
$self->{'var'} = \@return_data;
# $self->{'var'}= \$file;
}

1;

Thank you!

Olivier

 

[Sam Holden]

Hi there,

I'm playing with perl to understand how I could make modules.
Here is my "first module" ...And...It doesn't work.

This module should parse a CSV file an extract the appropriate columns
(it should return an array).

An array is not an array reference, your code and spec. disagree.

I call it with:
my $test= parse_file->new(FILE=>"./log/1fr.txt",AREA=>6);
my $arrayref=$test->getdata();

But it fails. I receive an error "could not open SCALAR(XXXX)";
it seems that FILE is transformed into a hexadecimal code ?
What is my mistake?

Your code doesn't output such a message. Not reporting the actual
behaviour makes it far less likely that people will be able to help.
Alternatively not posting the code you are actually running makes
is next to impossible that people will be able to help.

package parse_file;

Lowercase only names are used for pragmas by convention in perl,
use a different name.

use 5.001;

use strict;

# use warnings;

The warning you disabled by commenting out that line is
a bug in your code. Rather than telling perl to not bother
telling you what is wrong and asking here, why not read
what perl says and fix the problem.

It is insulting to be asked to do a job a machine can
do in a millisecond or so.

sub new {
my $self = {};
$self->{'FILE'} = undef;
$self->{AREA} = undef;
bless {$self};

That does not do what you think it does.

The warning you disabled above gives a massive hint as to what is wrong
with it. Read the warning.

Also nowhere do you use @_ so the arguments to your new sub
are going to be ignored, so the fact that you passed some
above indicates yet another error.

}

sub getdata
{
my $self;
my $file = $self->{FILE};
my $area = $self->{AREA};
my @data;
my @return_data;
open(MYFILE,'$file')|| die "cannot open $file: $!";

Those single quotes are wrong. Don't randomly place quote marks
in your perl code.

while(defined($a=<MYFILE>))
{
my @data=split(/\,/,$a);

, is not special in a regex and hence doesn't need escaping.

[snip the rest]

 

[Olivier Laurent]

It is insulting to be asked to do a job a machine can
do in a millisecond or so.

Well thanks, for Playing the guru teaching the poor mass. But my
problem wasn't your ego, my problem is how to make my first perl
module and having fun while doing it. I'm just asking advices, nobody
forces you to reply.

My problem is why a filename like "./log/test.txt" is transformed into
SCALAR(10CBAA) when it is sent through parse_file->new($filename)

warmings has been disabled because it gave nothing, I played around to
understand what's going on.

I will respect lowercase and stuff like that while I have something
relevant to share with the community. not for a stupid module made by
a complete beginner to learn object oriented coding in perl.



Olivier

 

[Sam Holden]

Well thanks, for Playing the guru teaching the poor mass. But my
problem wasn't your ego, my problem is how to make my first perl
module and having fun while doing it. I'm just asking advices, nobody
forces you to reply.

I gave you advice. I pointed out three problems. One exactly by
line and character, one via the warning perl will
generate (giving the line number and description of the
problem), and one by pointing out an omission.

My problem is why a filename like "./log/test.txt" is transformed into
SCALAR(10CBAA) when it is sent through parse_file->new($filename)

And as I said, it isn't because it doesn't. The code you posted does
not generate that output.

The file name isn't "sent through parse_file->new($filename)" since
all the arguments to new() are ignored.

warmings has been disabled because it gave nothing, I played around to
understand what's going on.

It pointed out a bug that will make the code not work.

That seems to be giving something to me.

But to each his own, if you want non-working code then that's
your business.

 

[gnari]

(e-mail address removed) (Sam Holden) wrote in message

Well thanks, for Playing the guru teaching the poor mass. But my
problem wasn't your ego, my problem is how to make my first perl
module and having fun while doing it. I'm just asking advices, nobody
forces you to reply.

[slightly reordered comments]

warmings has been disabled because it gave nothing, I played around to
understand what's going on.

you are taking this too personally. when you post a script
because of a problem, and we see no warnings enabled, it is not
worth it for us to look too closely at the code, because we would
have to start by cuttting and pasting your code into an editor,
add the warning, save it and run it , just to see if there are any
warnings. this often seems a bit too much work to do to help
someone who was too lazy to do it himself. now, if you had said
in your OP that 'it ran without warnings when enabled' or some such,
many of us probably would have trusted you on that,

My problem is why a filename like "./log/test.txt" is transformed into
SCALAR(10CBAA) when it is sent through parse_file->new($filename)

actually your code said:
my $test= parse_file->new(FILE=>"./log/1fr.txt",AREA=>6);
so you are calling parse_file($some_object_reference);

your main problem is that you do not seem to have grasped how
parameters are picked up by subs.

neither your new() , nor your parse_file seem to do anything
with any arguments sent.

I will respect lowercase and stuff like that while I have something
relevant to share with the community. not for a stupid module made by
a complete beginner to learn object oriented coding in perl.

you were asking for comments

in addition, let me add:
you should declare $a as lexical in parse_file():
my $a;

did you notice the comment Sam made about the single quotes in the
open() call.

gnari

 

[Tad McClellan]

But it fails. I receive

I don't get that when I run your code.

If we cannot duplicate the problem, we probably cannot solve the problem.

an error "could not open SCALAR(XXXX)";
it seems that FILE is transformed into a hexadecimal code ?

That is a stringified reference.

Somehow or another (we can't tell because the posted code does not
do what you say it does...) you have a reference to a scalar where
you were probably expecting a scalar itself.

What is my mistake?

You have multiple mistakes.

sub new {
my $self = {};
$self->{'FILE'} = undef;
$self->{AREA} = undef;
bless {$self};

^ ^
^ ^

Those should not be there.

new() does not access any arguments, yet you call it with arguments...

my $file = $self->{FILE};
my $area = $self->{AREA};

Those will always be undef, since your code never assigns anything
else to them...

my @data;
my @return_data;
open(MYFILE,'$file')|| die "cannot open $file: $!";

^ ^
^ ^

Those should not be there either.

# $self->{'var'}= \$file;

Now _there_ is a reference to a scalar.

Was that line uncommented when you saw the SCALAR(XXXX) output?



If you post a short and complete program *that we can run*
that illustrates your problem, we can help you fix it.

If you don't we can't.

Have you seen the Posting Guidelines that are posted here frequently?

 

[Richard Morse]

in addition, let me add:
you should declare $a as lexical in parse_file():
my $a;

Actually, isn't this one of those special cases where you want to use a
different variable name, because $a is involved in sorting? Or does it
not count because we're in a different package here?

Ricky

 

[gnari]

Actually, isn't this one of those special cases where you want to use a
different variable name, because $a is involved in sorting? Or does it
not count because we're in a different package here?

that is not the problem here, as no sorting was involved, but $a and $b
are brobably avoided to reduce confusion.
in this case, $ a was used as an undeclared global, but is not checked
by 'use strict' because of it's special status. had the variable name been
$aa, then an error would have been thrown by strict

gnari