Beefy Boxes and Bandwidth Generously Provided by pair Networks
Welcome to the Monastery
 
PerlMonks  

OO method wont parse

by jorba (Sexton)
on Aug 28, 2017 at 20:18 UTC ( #1198204=perlquestion: print w/replies, xml ) Need Help??

jorba has asked for the wisdom of the Perl Monks concerning the following question:

Getting back into perl after 15 years away and trying to learn the OO side of things.

Problem is that I'm doing something I shouldn't in the methods/subroutines, but I'm not sure what. Have looked at examples on the web that appear to do the same thing.

using activestate perl on windows

What I'm trying to achieve is to keep the dbi/sql object instantiated in the constructor by adding it to the hash that is the object (i.e. as a property) The subroutines can then grab that from the hash and use it, rather than having to re-create it.

Here's the code.

package AXSQL; # Our libraries use lib 'C:\Users\Jay\Desktop\SBS DEV\CODE\perl\Utilities'; use AXControl; use strict; use warnings; use DBI; # SQL object removes the need to handle the complexity of SQL connecti +ons and processing in individual programs package SQL; sub new # Constructor { my $class; my $sql; my @bindparms; my $Rowcount; my $Status; my $cntl; my $db; my $start; my $end; my $posn; my $SQLStr; my $host; my $Properties; @bindparms = $_; $class = shift @bindparms; $cntl = shift @bindparms; $sql = shift @bindparms; $db = $cntl->{"SysHandle"}; if (substr($sql,1,4) eq 'SQL.') { # Get SQL from the file directories $sql = $db->prepare("SELECT SQLTEXT FROM SQLDEFN WHERE SQLID = + substr($sql,5)") or die 'Couldnt run sql: '. $db->errstr; $sql->execute(); ($SQLStr) = $sql->fetchrow_array(); if ($sql->rows == 0) { return "SQL does not exist" } } #Resolve host variables $start = 1; $posn = index($SQLStr, ":"); while ($posn != -1) { $end =index(substr($SQLStr,&posn, length($SQLStr) - $posn + 1) +, " "); $host = substr($SQLStr, $posn, $end - $posn); $SQLStr =~ s/$host/@bindparms [ substr($host,2,length($host) - +1)]/g; $posn = index($SQLStr, ":"); } $sql = $db->prepare($SQLStr); $sql->execute(); $Properties= {}; bless ($Properties, $class); $Properties->{'SQL'} = $sql; $Properties->{'String'} = $SQLStr; } Sub Fetch { my $class = shift; my $sql = $class->{SQL}; #return $sql->fetchrow_array(); } Sub RowCount { $class = shift; $sql = $class->{SQL}; return $sql->rows; } 1; Here's the error messages C:\Users\Jay\Desktop\SBS DEV\CODE\perl\Utilities>perl -c AXSQL.pm Semicolon seems to be missing at AXSQL.pm line 81. Number found where operator expected at AXSQL.pm line 90, near "1" (Missing semicolon on previous line?) syntax error at AXSQL.pm line 78, near "my " Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 78. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 78. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 85. Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 86. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 86. Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 87. syntax error at AXSQL.pm line 88, near "}" AXSQL.pm had compilation errors.
Here are the error messages
C:\Users\Jay\Desktop\SBS DEV\CODE\perl\Utilities>perl -c AXSQL.pm Semicolon seems to be missing at AXSQL.pm line 81. Number found where operator expected at AXSQL.pm line 90, near "1" (Missing semicolon on previous line?) syntax error at AXSQL.pm line 78, near "my " Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 78. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 78. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 85. Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 86. Global symbol "$class" requires explicit package name (did you forget +to declare "my $class"?) at AXSQL.pm line 86. Global symbol "$sql" requires explicit package name (did you forget to + declare " my $sql"?) at AXSQL.pm line 87. syntax error at AXSQL.pm line 88, near "}" AXSQL.pm had compilation errors.
All insights greatly appreciated

Replies are listed 'Best First'.
Re: OO method wont parse
by toolic (Bishop) on Aug 28, 2017 at 20:27 UTC
    The errors disappear for me when I use all lower-case for the "sub" keyword (not "Sub") and when I declare your variables with my in the RowCount sub:
    sub Fetch { my $class = shift; my $sql = $class->{SQL}; #return $sql->fetchrow_array(); } sub RowCount { my $class = shift; my $sql = $class->{SQL}; return $sql->rows; }

    My text editor with syntax highlighting clearly showed the Sub as not being a keyword.

Re: OO method wont parse
by talexb (Chancellor) on Aug 28, 2017 at 20:52 UTC
      my $class; my $sql; my @bindparms; my $Rowcount; my $Status; my $cntl; my $db; my $start; my $end; my $posn; my $SQLStr; my $host; my $Properties; @bindparms = $_; $class = shift @bindparms; $cntl = shift @bindparms; $sql = shift @bindparms;

    A couple of stylistic points, from the above sub-set of your code.

    • It's neater to declare variables in a list, so as to make better use of space. So, my ( $class, $sql, .. ); is much cleaner than what you have, which is one variable declaration per line. I would probably group related variables together -- they don't all have to be on a single line.
    • You don't have to group variable declarations all at the beginning of a block, as we used to do in C. (I believe that was because they needed to be allocated on the stack by the compiler.) Declare variables when you're about to use them.
    • I won't comment on the choice of camelCase versus snake_case, but it is a good idea to be consistent. You've got some variable names that are lower case, and some that are Capital case (initial capital letter, followed by lower case).
    • You set @bindparams to @_, then use shift to initialize the three variables $class, $cntl and $sql. It's probably easier and cleaner just to say my ( $class, $cntl, $sql ) = @_;
    • Whenever you're passing arguments into a method, you should do validation on the arguments. At the very least, check each input to find out a) Is it defined? and b) Does it have a reasonable value? Only move forward if the input arguments are absolutely fine; otherwise, output a useful error message and return (or die/croak, whichever is appropriate).

    Alex / talexb / Toronto

    Thanks PJ. We owe you so much. Groklaw -- RIP -- 2003 to 2013.

      You don't have to group variable declarations all at the beginning of a block, as we used to do in C. (I believe that was because they needed to be allocated on the stack by the compiler.) Declare variables when you're about to use them.

      Just a little bit of bean counting: C99 allows to declare variables where you need them. Today's C compilers have become smart enough. Here's a silly example:

      /tmp>cat c99.c #include <stdio.h> #include <string.h> int main(int argc, char ** argv) { char buf1[10]; snprintf(buf1,sizeof(buf1),"%d",argc); int n=0; for (int i=0; i<argc; i++) { int len=strlen(argv[i]); len++; n+=len; } char buf2[10]; snprintf(buf2,sizeof(buf2),"%d",n); char buf3[80]; snprintf(buf3,sizeof(buf3),"%s args using %s bytes",buf1,buf2) +; puts(buf3); return 0; } /tmp>CFLAGS="-std=c99 -pedantic -Wall" make c99 cc -std=c99 -pedantic -Wall c99.c -o c99 /tmp>./c99 foo bar baz 4 args using 18 bytes /tmp>

      Alexander

      --
      Today I will gladly share my knowledge and experience, for there are no sweeter words than "I told you so". ;-)

        Thanks -- my experience in this matter comes from looking at the output from a VMS cross-compiler that generated 68000 assembler in the mid-80's. I imagined that this shortcoming would have been solved since then. :)

        Alex / talexb / Toronto

        Thanks PJ. We owe you so much. Groklaw -- RIP -- 2003 to 2013.

Re: OO method wont parse
by Corion (Patriarch) on Aug 28, 2017 at 20:30 UTC

    The keyword is sub with a lower case s, not an upper case S.

    To find this syntax error, I removed complete subroutines from your code until the error disappeared, then added the last subroutine back in.

    Then I removed statements from the code until I saw the error.

Re: OO method wont parse
by LanX (Sage) on Aug 28, 2017 at 20:35 UTC
Re: OO method wont parse
by Mr. Muskrat (Canon) on Aug 29, 2017 at 17:37 UTC

    What I'm trying to achieve is to keep the dbi/sql object instantiated in the constructor by adding it to the hash that is the object (i.e. as a property) The subroutines can then grab that from the hash and use it, rather than having to re-create it.

    Is this app single threaded? If not, be sure to read up on DBI - Threads and Thread Safety.

    Also, how are you dealing with disconnected database handles? Have you seen DBIx::Connector?

Re: OO method wont parse
by jorba (Sexton) on Aug 28, 2017 at 20:54 UTC
    That's solved it. Knew it was something dumb ;o) Many thanx J.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://1198204]
Approved by stevieb
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others pondering the Monastery: (3)
As of 2022-05-21 13:23 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Do you prefer to work remotely?



    Results (76 votes). Check out past polls.

    Notices?