byterock

Still in re-factor mode here at the Moose-pen.

Yesterday I had quite the success in taking a little more of the logic out of the DAD side side of things now the DAD writes not longer have to check to see if a predicate in a condition or filter has miss-matched parentheses or is missing at least the the default 'and' condition.

Before I move on I want to squish this little warning

Use of uninitialized value in join or string at...

which is really just sloppy programming on my part as this

$predicate_clause .= join( " ",

                $predicate->left->view . "." . $predicate->left->name,

                $predicate->operator,

                "'" . $predicate->right->value . "'",

--                 $predicate->condition);

++                ($predicate->condition) ? $predicate->condition : "" );

That gets me right into the first bit of re-factoring for today getting rid of that testing kludge where I have hard coded the left side of a predicate to be a value. On top of this I do not even take into account any open or closed parentheses so that should go in there as well. So out goes all of that.

Before I get too far along I winn need a test case, so I created 20_where_basic.t. Not to bore you too much with just code the real hart of it is;

my $da     = Database::Accessor->new($in_hash);

ok($da->retrieve( $utils->connect() ),"selected something");

ok($da->result()->query() eq "SELECT people.first_name, people.last_name, people.user_id FROM people WHERE ( people.first_name = ? AND people.last_name = ? )","Select SQL correct");

The $in_hash is just defined DA which you can go an look up if you want the important part in that is I set the 'da_compose_only' to true so this query never goes out to a DB it just sets the 'result' attribute where I can see what SQL the Driver::DBI generated.

Now when I first ran it I get a fail as the output of the SQL is

SELECT people.first_name ,people.last_name ,people.user_id  FROM people WHERE  people.first_name = 'test1' people.last_name = 'test2' AND

So I have a little fixing up to do and here is the first round of fixes;

sub _predicate_clause {

    my $self = shift;

    my ( $clause_type, $conditions ) = @_;

    my $clause           = " $clause_type";

    my $predicate_clause = "";


    foreach my $condition ( @{$conditions} ) {

        foreach my $predicate ( @{ $condition->{predicates} } ) {

          $predicate_clause .= $self->_predicate_sql($predicate);

        }

    }

    $self->da_warn( "_predicate_clause",

        $clause_type . " clause='$predicate_clause'" )

      if $self->da_warning() >= 5;

    return join( " ",$clause, $predicate_clause );

}

sub _predicate_sql {

    my $self = shift;

    my ($predicate) = @_;

    my $clause =  "";

       $clause .= " "

              .$predicate->condition()

              . " " 

     if ($predicate->condition());

    $clause .= Database::Accessor::Driver::DBI::SQL::OPEN_PARENS

      if ( $predicate->open_parenthes() );

    if (Database::Accessor::Driver::DBI::SQL::SIMPLE_OPERATORS->{ $predicate->operator }){

       $clause .= $self->_element_sql($predicate->left)

               . $predicate->operator

               . $self->_element_sql($predicate->right);

    }

   $clause .= Database::Accessor::Driver::DBI::SQL::CLOSE_PARENS

      if ( $predicate->close_parenthes() );

   $self->da_warn( "_predicate_sql",

                   " clause='$clause'" )

      if $self->da_warning() >= 6;

    return $clause;

}

sub _element_sql {

  my $self = shift;

  my ($element) = shift;

  if (ref($element) eq 'Database::Accessor::Param'){

    $self->add_param($element);

    return Database::Accessor::Driver::DBI::SQL::PARAM;

  }

  else {

    return $element->view

           ."."

           .$element->name;

  }

}

In the above I have re-factored the code by creating two new subs '_predicate_sql' and '_element_sql' and here are the highlights.
In _predicate_sql I check to see what operator is coming in before I create the 'SQL' in this case I am checking for simple operators ie, '=','>','!=' etc. Now I have a place to add in the other more complex operators like 'IN' or 'BETWEEN' by extending the initial 'if' statement. In this sub I take a little lesson from my HP-10C reverse polish notation days and add the conditional first (if it exists) then add in the rest of the SQL. Finally as I know my parentheses are balanced all I need to do is add them in if they exists.

In the '_element_sql' I check each '$element' coming in and see what is it and do the appropriate SQL output. I can now also expand this if to allow other things besides a param or an element. I think I already have Array stubbed in elsewhere. As part of this re-factoring I reused this sub in other places as well.

Now the only thing to get rid of is a “ ,” rather than a “, “ in the field clause on select and I accomplished this with this little change;

sub _element_sql {

  my $self = shift;

--  my ($element) = shift;

++  my ($element,$use_alias) = shift;

…

    my $sql = $element->view

           ."."

           .$element->name;

    $sql .= join(" ",

                 Database::Accessor::Driver::DBI::SQL::AS, 

                 $element->alias())

       if ($element->alias and $use_alias );

    return $sql;

…

Now I can call the element_sql with that $use_alias flag where I need it and with a few more little changes moving the odd “,” and “ “ about I finally get

ok 1 - selected something

ok 2 - Select SQL correct

So not a bad day's work;

by the way this is my 180th day in a row posting (big deal) so happy Moose times;