Beefy Boxes and Bandwidth Generously Provided by pair Networks
laziness, impatience, and hubris
 
PerlMonks  

PerlOO what i am doing???

by seekperlwisdom (Acolyte)
on Aug 11, 2012 at 20:22 UTC ( [id://986920]=perlquestion: print w/replies, xml ) Need Help??

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

All Monks, Can you please help me clear my mind?
package christianDatabase; use strict; use warnings; use DBI; use Time::Piece::MySQL; my $timeNow = localtime; my $date = $timeNow->mysql_datetime; my $dbh = DBI->connect("dbi:mysql:christianDatabase", "xxxacctxxx" , " +xxxpassxxx") or die; sub new { my($class)= @_; bless{ },$class; } sub add_comment{ my($self, $comment, $username,) } sub make_comment_record{ } sub check_comment_record{ my($self, $MNid) = @_; $self->{_MNid} = $MNid; my $sql = "select MNref from comment_record where MNid=" . "\"$sel +f->{_MNid}\""; my $ccr = $dbh->prepare($sql); $ccr->execute() or die "$!"; my $cct = $ccr->fetchrow_array; return $cct; } sub add_new_news{#news Heading, full text, news photos, url my ($self, $newsHeading, $fullText, $NewsPhoto, $url) = @_; $self->{_news_heading} = $newsHeading; $self->{_news_text} = $fullText; $self->{_news_photo} = $NewsPhoto || ''; $self->{_news_url} = $url; my $newsid = &new_News(); my $sql = "insert into news (news_heading, full_text, news_photos, url, date, news +ID) values(\"$self->{_news_heading}\", \"$self->{_news_tex +t}\", \"$self->{_news_photo}\", \"$self->{_news_url}\", \"$date\", \" +$newsid\")"; print "\n\n$sql\n\n"; my $addNews = $dbh->prepare($sql); $addNews->execute() or die "$!"; return "News added"; }
I am trying to make a module for project. i am not blessing variable . though i am creating varibles in methods. It works fine i just want to know if anything is wrong with it. i need to add more methods and new variables to process. thanks

Replies are listed 'Best First'.
Re: PerlOO what i am doing???
by tobyink (Canon) on Aug 12, 2012 at 07:08 UTC

    You are not currently doing object-oriented programming. You are doing something that might superficially resemble OOP, but it's not OOP.

    If you want to do OOP in Perl, I'd strongly suggest Moose or one of its friends (Mouse, Moo or Mo). These toolkits hide away much of the boilerplate code associated with Perl OOP, and allow you to concentrate on the business of writing and using your classes.

    OOP aside, I'd say that your biggest problem though is the fact that you're not using DBI's prepared statements properly. Don't do this:

    my $sth = $dbh->prepare("SELECT * FROM foo WHERE bar='$baz'"); $sth->execute;

    Do this (thanks Anonymous Monk below for the correction):

    my $sth = $dbh->prepare("SELECT * FROM foo WHERE bar=?"); $sth->execute($baz);

    This allows DBI to handle escaping issues for you, and in many cases optimize multiple calls of execute.

    Anyhow, here's my rewrite of your package using Moose and prepared statements...

    perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'

      ... WHERE bar = ?, not ... WHERE bar = '?'

    A reply falls below the community's threshold of quality. You may see it by logging in.
Re: PerlOO what i am doing???
by xyzzy (Pilgrim) on Aug 12, 2012 at 00:37 UTC

    Before anyone bothers to look at this, you might want to remove your plaintext password from the code. Not that anyone will hack your database, but it is very common for people to reuse one or several passwords for all their various logins. It's obviously not relevant to making your code work, so you really should replace it with "dbpassword" or "xxxxxxxxx". And again, I'm not saying that anyone in this community will try to take advantage of this, but it's just a good idea to replace any sensitive information with junk before posting it to a publicly accessible web site that is indexed by major search engines.


    $,=qq.\n.;print q.\/\/____\/.,q./\ \ / / \\.,q.    /_/__.,q..
    Happy, sober, smart: pick two.
      thanks but these are not real. though i should have used 'dbuser' rather than 'xxxaccountxxx'

        When I made my first reply, it was still 'root', and the other was also something else that I won't repeat but I do remember ;)


        $,=qq.\n.;print q.\/\/____\/.,q./\ \ / / \\.,q.    /_/__.,q..
        Happy, sober, smart: pick two.
Re: PerlOO what i am doing???
by xyzzy (Pilgrim) on Aug 12, 2012 at 01:23 UTC

    What are you doing???

    You have a constructor that doesn't do anything. Normally it would perform actions necessary to set up the object, for instance if your object was the database, the constructor might attempt to open a connection and store a handle to the database, as well as provide a mechanism for catching and reporting errors that occur.

    You have a bunch of unfinished and unstarted methods. And then the methods that you do have are procedural and don't really fit with the OO paradigm.

    Before you go any further, you should look up information about Object Oriented Programming, learn about abstraction, encapsulation, the difference between classes and objects, the relationship between objects and methods, etc. Then, only when you understand how OOP works, if you are working on a project that relies heavily on a database, I highly recommend DBIx::Class, a module that provides an object-oriented interface to a relational database (it is compatible with many backends, including MySQL). This will eliminate the need for SQL and it can generate classes with accessors and relationship mapping for all of your tables in a way that is intuitive and easy to understand. Right now your code, aside from being messy, wasteful, and mind-bogglingly obtuse, is vulnerable to SQL injection attacks (what will happen if I call $database->check_comment_record('1; drop table comment_record')?). Using DBIx::Class will solve these problems for you without you having to know anything about database security. Creating new records is as easy as

    $schema->resultset('NewsPost')->create({ heading => "Llama thief still at large!", text => "Police are investigating leads but have refused to comment +on possible suspects...", photos => [{ img => 'llamas_01.png', caption => 'Five llamas have been confirmed missing since Tuesda +y (File photo)', },{ img => 'llamas_02.png', caption => 'Police Commissioner Turgidson, shown here making a p +ress statement Wednesday morning', }], url => '/news/llamas', })

    You have a lot of reading to do before you can even try to do something like this. Wikipedia is an OK place to start. There are certain websites where you can obtain pdf copies of books about database programming. The CPAN documentation for DBI modules will be helpful if you sit down and read it very carefully. When you have enough of an idea about what you're doing to ask an actual question you can come back. The community here would be glad to help you, but only if you actually show us that you are someone who knows how to RTFM, use Google, and that you can figure basic things out on your own before asking for help with a specific issue or problem.


    $,=qq.\n.;print q.\/\/____\/.,q./\ \ / / \\.,q.    /_/__.,q..
    Happy, sober, smart: pick two.
    A reply falls below the community's threshold of quality. You may see it by logging in.
Re: PerlOO what i am doing???
by chacham (Prior) on Aug 12, 2012 at 20:32 UTC

    As a side note, building SQL strings is dynamic SQL. It is inefficient, pone to error, and allows for SQL injection.

    As you're using prepare and execute on the actual statements, you can use placeholders and pass the variable's in a hash (the optional second parameter to execute), which is the first line of defense against SQL injection and more efficient.

    That is, instead of:

    my $sql = "select MNref from comment_record where MNid=" . "\"$self->{ +_MNid}\""; my $ccr = $dbh->prepare($sql); $ccr->execute() or die "$!";

    use

    my $sql = "select MNref from comment_record where MNid= ?"; my $ccr = $dbh->prepare($sql); $ccr->execute($self->{_MNid}) or die $ccr->errstr;

    It's also a lot easier to read. Note, also, return DBI's error, instead of just $!.

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others having an uproarious good time at the Monastery: (5)
As of 2024-03-29 12:33 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found