pas-dev

[Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Kyle R . B. <mo...@vo...>]

First proposal: logging

Justin and I recently worked on a project that used the Apache/Jakarta
projects' Log4j logging package.  We absolutely loved it.

There is a CPAN package that says it's a Perl version of Log4j, it's
called Log::Dispatch and Log::Dispatch::Config.  I'd like to propose
that we remvoe the current logging system and implement the Log::Dispatch.
It'll be alot more configurable as far as log levels go, and it'll be alot
easier to have it log to other places -- you can configure a dispatch
to go to a file, to the sys log, or you can configure it to log fatal
errors as email (I think), plus it provides an api for you to implement
your own logging.


Second proposal: Exception handling

I'd like to propse that we get rid of throw() in favor of using the CPAN 
module Error.pm.  That will give us typed exceptions.  Maybe we can even
update the request handler to catch the permissions problems that Doug
just ran into and report them instead of silently failing. 


Third proposal: get rid of _gss()/_get_set_scalar()

I'd like to propose that we get rid of these in favor of makeGetSetMethods().
Example:

  package MyObj;
  use strict;
  use warnings;
  use Org::Bgw::Base; # or any object derived from Base::Common
  our @ISA = qw( Org::Bgw::Base );
  MyObj->makeGetSetMethods( qw( foo bar qux ) );

  sub method1
  {
    my($self) = @_;
    $self->foo('this');
    $self->bar( $self->qux() );
  }

It creats the get/set function at run-time and obviates the extra level
of function call to _gss().  It also makes the code cleaner to look at.


The first two proposals raise the issue of external dependencies...should
we start depeding on external modules?  I guess in some sense we already
do -- some apapche modules, the mysql driver, and soon the pg driver. 


What do the other developers think?

k

-- 

------------------------------------------------------------------------------
Wisdom and Compassion are inseparable.
        -- Christmas Humphreys
mo...@vo...                            http://www.voicenet.com/~mortis
------------------------------------------------------------------------------

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Mental <me...@ne...>]

On Fri, 2002-05-17 at 10:41, Kyle R . Burton wrote:
> First proposal: logging
> 
> Justin and I recently worked on a project that used the Apache/Jakarta
> projects' Log4j logging package.  We absolutely loved it.
> 

Sounds reasonable. I often get the feeling we're following java at this
point. I suppose we are. Still, its a good idea. I can see it being
useful.


> Second proposal: Exception handling
> 
> I'd like to propse that we get rid of throw() in favor of using the CPAN 
> module Error.pm.  That will give us typed exceptions.  Maybe we can even
> update the request handler to catch the permissions problems that Doug
> just ran into and report them instead of silently failing. 
> 
> 

It didnt silently fail, it was in pas.log, just not in the browser. But
yes. Why reinvent the wheel? Error.pm does indeed have some nice
to-haves. Leverage CPAN and note it in the INSTALL.



> Third proposal: get rid of _gss()/_get_set_scalar()
> 

I agree for different reasons. I feel generic get/set functions are
error prone. Except in the most basic situations they're probably not
what you want. If its just a string to display, remember it. 

> I'd like to propose that we get rid of these in favor of makeGetSetMethods().
> Example:
> 
>   package MyObj;
>   use strict;
>   use warnings;
>   use Org::Bgw::Base; # or any object derived from Base::Common
>   our @ISA = qw( Org::Bgw::Base );
>   MyObj->makeGetSetMethods( qw( foo bar qux ) );
> 

The context of this will determine if I like it or not. 

<SNIP>

> It creats the get/set function at run-time and obviates the extra level
> of function call to _gss().  It also makes the code cleaner to look at.
> 

They also dont catch errors. And thats a big issue with me right now.
For blobby unstructured data, rock on. But for db stuff, this causes
issues further down the road. 

> 
> The first two proposals raise the issue of external dependencies...should
> we start depeding on external modules?  I guess in some sense we already
> do -- some apapche modules, the mysql driver, and soon the pg driver. 
> 

It also raises the issue of backwards compatibility. Will this cause
code to break? If its transparent to 'old style' stuff, fine. If not, is
this a new branch/release major version/whatever? Just asking. They're
good ideas.

> 
> What do the other developers think?
> 

I think I'm seeing the issues with DB abstraction so I'm doing a first
round of 'just make it work' coding. 

I do have a fourth proposal.

four: Modify object generator to gen smarter objects.

I think we can make db records be smart enough that when I call a get
set with bad data, the object catches it and yells.

Thus, if an object is generated and has a get/set for a number 2 column,
and  call $obj->foo('string'), it shouldn't like it. My argument for
doing this is its easier to do when generating an object than it is when
trying to store an object. Its too error prone. And getting exceptions
on insert doesnt always tell me _which_ value was too large for a
column. The side effect of this is that we're probably going to have to
subclass objectgenerator for every database we support to deal with
their different personalitys. Since we currently only support MySQL,
this isnt a huge amount of work. However, going forward, the question of
access to every possible backend becomes an issue.

What do you think? This lack of data checking in generated objects is
making me seriously rethink their merits. Sure it takes longer, but I
can write an object by hand that will not accept bad data. I'd rather
spend the time up front writing the object, than debugging on insert
errors. 


-- 
Mental (Me...@Ne...)

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Kyle R . B. <mo...@vo...>]

> Sounds reasonable. I often get the feeling we're following java at this
> point. I suppose we are. Still, its a good idea. I can see it being
> useful.

Yea and no.  We're modeling the good aspects of JSP and the servelt model
as those aspects translate and work well in the Perl/mod_perl arena.

The log4j package is not Java, it is just a really good logging package that
happens to have been first written in Java.  Now it's available for Perl,
and it's good, so I say we use it.

That's 2 +1s, anyone else have an opinion?

> > Second proposal: Exception handling
>
> It didnt silently fail, it was in pas.log, just not in the browser. But
> yes. Why reinvent the wheel? Error.pm does indeed have some nice
> to-haves. Leverage CPAN and note it in the INSTALL.

That's 2 +1s, anyone else have an opinion?

> > Third proposal: get rid of _gss()/_get_set_scalar()
> 
> I agree for different reasons. I feel generic get/set functions are
> error prone. Except in the most basic situations they're probably not
> what you want. If its just a string to display, remember it. 

What errors?  Their purpose is to keep you from writing the ever present
standard get/set functions.  That's all - if you need more functionality,
then don't use it, write the get/set yourself and add in the desired
behavior.  Just because it's there doesn't mean you have to use it, it's
optional so you don't have to use it if it's not appropriate.

> The context of this will determine if I like it or not. 

The only difference between makeGetSetMethods() and _gss() is that
_gss is slower, and you have to write more code, which opens up possibilities
for errors.  That's it.

I know I've doen this before:

  sub foo {shift->_gss('foo');}

and wasted time debugging it, when it should have been:

  sub foo {shift->_gss('foo',@_);}

makeGetSet is better for these kinds of programmer errors.  It is also
faster because there is 1 level of function call involved, not two.  It's
also faster, because you're not shifting @_ to call your parent's _gss()
method.

The only faster thing I know of (besides going back to direct hash access)
is to use pseudo hashes, and I'm not going to suggest that for a few 
reasons - it's documented as experimental, and in Larry's most recent 
state of the Onion, he talks about getting rid of them.

> They also dont catch errors. And thats a big issue with me right now.
> For blobby unstructured data, rock on. But for db stuff, this causes
> issues further down the road. 

You talk about this more below, so I'll comment on it there.

> It also raises the issue of backwards compatibility. Will this cause
> code to break? If its transparent to 'old style' stuff, fine. If not, is
> this a new branch/release major version/whatever? Just asking. They're
> good ideas.

Most of it will not.  Some of the codebase is already using makeGetSetMethods.
The change to Exception should be drastic - we should gut the codebase of
all instances of $self->throw().  The logging change should also be drastic
in the same way.  

> I think I'm seeing the issues with DB abstraction so I'm doing a first
> round of 'just make it work' coding. 
> 
> I do have a fourth proposal.
> 
> four: Modify object generator to gen smarter objects.

I personaly like this.  On a project Justin and I just did, and in 
some conversations I had with Andy, we want to generate the proper
SQL staemetns for SELECT,INSERT,UPDATE and DELETE.  That way they
can deferr to a parent object for these operatsions -- the generated
record objects will be smart enough to know how to perform these operations
on themselves.  For the project Justin and I worked on, we only wrote
sql statements for reporting purposes.  Of the few hundred sql statemetns
we would have had to write for the 30 or so database tables, they all
got handled implicitly by the code in the generated objects.  That saved
us no end of time.  Of course it's not necessarily the most optimized
SQL code, but we optmized afterwards when we saw hotspots in the 
application, which ended up being about 2 places.


> I think we can make db records be smart enough that when I call a get
> set with bad data, the object catches it and yells.
> 
> Thus, if an object is generated and has a get/set for a number 2 column,
> and  call $obj->foo('string'), it shouldn't like it. My argument for
> doing this is its easier to do when generating an object than it is when
> trying to store an object. Its too error prone. And getting exceptions
> on insert doesnt always tell me _which_ value was too large for a
> column. The side effect of this is that we're probably going to have to
> subclass objectgenerator for every database we support to deal with
> their different personalitys. Since we currently only support MySQL,
> this isnt a huge amount of work. However, going forward, the question of
> access to every possible backend becomes an issue.

I like the idea of adding in type checking or validation.  We could
either make that a switch on the generator, or a run-time swtich so
you could use it in development, and turn it off for production for
speed/performance reasons.

> What do you think? This lack of data checking in generated objects is
> making me seriously rethink their merits. Sure it takes longer, but I
> can write an object by hand that will not accept bad data. I'd rather
> spend the time up front writing the object, than debugging on insert
> errors. 

I have a new proposal for the generator.  The current appraoch sucks.
I don't think we need to sub-class the generator, we need to throw it away.  
As It is currently written it is a bad approach (too much maintenence).

I propose that we do what Justin and I did for the last project at HMS:
use PSP to generate the objects.  It's _soo_ much easier.  We write
a driver program:

  dbRecGenerator.pl --dbiDsn="dsnString" --dbUser=user --dbPass=pass \
     --dbTable=TABLE_NAME --genClassName=Some::Class::RecName \
     --pspTemplate=recGen.psp
     
The driver program connects to the database, and puts togeather a data
structure (or better yet, an object) that has all of the data about the
table.  Then the PSP looks like this:

  <%
    my $tableInfo = $self->query()->param('tableInfo');
    my $packageName = $self->query()->param('packageName');
  %>package <%= $packageName %>;
  use strict;
  use warnings;
  <%
    foreach my $use ( @someListOfObjectsToUse ) {
  %>
  use <%= $use %>;
  <%
    }
  %>
  our @ISA = qw( <%= @ourParentList %> );

  <% foreach my $colInfo ( $tableInfo->columns() ) { %>
  sub <%= $colInfo->functionName() %>
  {
    my($self) = @_;
    if( @_ > 2 ) { # a set
      $self->validate<%= $colInfo->functionName() %>($_[1]);
      return $self->setVal(x,y,z); # something like that
    }

    # put get code here
  }

  sub validate<%= $colInfo->functionName() %>
  {
    my($self,$val) = @_;
    # do your validation based on the col type and precision here...
  }
  <% } %>

  sub recInsertSql
  {
    # return sql for a basic insert
  }
  # also do a select, update, and delete...

  1;

Does this make sense?  Then you can just substitute out the particular
psp template that generates the recod objects in the way you like it to.

The templates will be infinitly easier to create and maintain than the
current borked generator design.

Comments?

Kyle

-- 

------------------------------------------------------------------------------
Wisdom and Compassion are inseparable.
        -- Christmas Humphreys
mo...@vo...                            http://www.voicenet.com/~mortis
------------------------------------------------------------------------------

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Mental <me...@ne...>]

On Fri, 2002-05-17 at 11:43, Kyle R . Burton wrote:

> Does this make sense?  Then you can just substitute out the particular
> psp template that generates the recod objects in the way you like it to.
> 
> The templates will be infinitly easier to create and maintain than the
> current borked generator design.
> 

+2. Dont worry if someone else doesnt like it. I like it enough for both
of us. :)


-- 
Mental (Me...@Ne...)

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Kyle R . B. <mo...@vo...>]

> > Does this make sense?  Then you can just substitute out the particular
> > psp template that generates the recod objects in the way you like it to.
> > 
> > The templates will be infinitly easier to create and maintain than the
> > current borked generator design.
> > 
> 
> +2. Dont worry if someone else doesnt like it. I like it enough for both
> of us. :)

Hey, we each only get 1 vote.  This might be a meritocracy, but we're going
to run it as much like a democracy as possible.

1 vote per develpoer.

So thats 2 in favor of PSP templates for db record object generation and 0
against.

I think we need 3 for a majority.


k


-- 

------------------------------------------------------------------------------
Wisdom and Compassion are inseparable.
        -- Christmas Humphreys
mo...@vo...                            http://www.voicenet.com/~mortis
------------------------------------------------------------------------------

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Justin B. <ju...@le...>]

Kyle R . Burton wrote:
>>>Does this make sense?  Then you can just substitute out the particular
>>>psp template that generates the recod objects in the way you like it to.
>>>
>>>The templates will be infinitly easier to create and maintain than the
>>>current borked generator design.
>>>
>>
>>+2. Dont worry if someone else doesnt like it. I like it enough for both
>>of us. :)
> 
> 
> Hey, we each only get 1 vote.  This might be a meritocracy, but we're going
> to run it as much like a democracy as possible.
> 
> 1 vote per develpoer.
> 
> So thats 2 in favor of PSP templates for db record object generation and 0
> against.
> 
> I think we need 3 for a majority.

didn't you get my rash of +1s?

+1 for PSP templates for DB record obj gen.
+1 for log4j (it's big pimpin'!)
+1 for TestCase
+1 for Error.pm
+1 for Jason (so he can vote twice this one time)
+1 for some kind of Base SQL object and have PostgreSQL & MySQL derive from it.
+1 to just have a PostgreSQL plugin.
+1 for makeGetSet() altho in a web application, we really haven't seen
the performance hit of gss() but it does clean up some stuff.
+1 for type checking.  i guess we can have different makeGetSets... 
makeGetSetString(), makeGetSetInt(), makeGetSetFloat(), etc.
-1 for Kyle for not letting both of Jason's brain cells vote.
+1 for improving security

did i cover them all?

justin

Re: [Pas-dev] Error.pm and Log::Dispatch/Log::Dispatch::Config

[Douglas R. <dou...@mi...>]

Kyle R . Burton wrote:
>>Sounds reasonable. I often get the feeling we're following java at this
>>point. I suppose we are. Still, its a good idea. I can see it being
>>useful.
> 
> 
> Yea and no.  We're modeling the good aspects of JSP and the servelt model
> as those aspects translate and work well in the Perl/mod_perl arena.
> 
> The log4j package is not Java, it is just a really good logging package that
> happens to have been first written in Java.  Now it's available for Perl,
> and it's good, so I say we use it.
> 
> That's 2 +1s, anyone else have an opinion?

sounds good.  +1.

>>>Second proposal: Exception handling
>>
>>It didnt silently fail, it was in pas.log, just not in the browser. But
>>yes. Why reinvent the wheel? Error.pm does indeed have some nice
>>to-haves. Leverage CPAN and note it in the INSTALL.
> 
> 
> That's 2 +1s, anyone else have an opinion?

also good.  yah.  +1.

>>>Third proposal: get rid of _gss()/_get_set_scalar()
>>
>>I agree for different reasons. I feel generic get/set functions are
>>error prone. Except in the most basic situations they're probably not
>>what you want. If its just a string to display, remember it. 
> 
> 
> What errors?  Their purpose is to keep you from writing the ever present
> standard get/set functions.  That's all - if you need more functionality,
> then don't use it, write the get/set yourself and add in the desired
> behavior.  Just because it's there doesn't mean you have to use it, it's
> optional so you don't have to use it if it's not appropriate.
> 
> 
>>The context of this will determine if I like it or not. 
> 
> 
> The only difference between makeGetSetMethods() and _gss() is that
> _gss is slower, and you have to write more code, which opens up possibilities
> for errors.  That's it.
> 
> I know I've doen this before:
> 
>   sub foo {shift->_gss('foo');}
> 
> and wasted time debugging it, when it should have been:
> 
>   sub foo {shift->_gss('foo',@_);}
> 
> makeGetSet is better for these kinds of programmer errors.  It is also
> faster because there is 1 level of function call involved, not two.  It's
> also faster, because you're not shifting @_ to call your parent's _gss()
> method.

i'd have to think about it some more, but makeGetSet does sound useful.  +1.

<snip>

>>I do have a fourth proposal.
>>
>>four: Modify object generator to gen smarter objects.
> 
> 
> I personaly like this.  On a project Justin and I just did, and in 
> some conversations I had with Andy, we want to generate the proper
> SQL staemetns for SELECT,INSERT,UPDATE and DELETE.  That way they
> can deferr to a parent object for these operatsions -- the generated
> record objects will be smart enough to know how to perform these operations
> on themselves.  For the project Justin and I worked on, we only wrote
> sql statements for reporting purposes.  Of the few hundred sql statemetns
> we would have had to write for the 30 or so database tables, they all
> got handled implicitly by the code in the generated objects.  That saved
> us no end of time.  Of course it's not necessarily the most optimized
> SQL code, but we optmized afterwards when we saw hotspots in the 
> application, which ended up being about 2 places.
> 
> 
> 
>>I think we can make db records be smart enough that when I call a get
>>set with bad data, the object catches it and yells.
>>
>>Thus, if an object is generated and has a get/set for a number 2 column,
>>and  call $obj->foo('string'), it shouldn't like it. My argument for
>>doing this is its easier to do when generating an object than it is when
>>trying to store an object. Its too error prone. And getting exceptions
>>on insert doesnt always tell me _which_ value was too large for a
>>column. The side effect of this is that we're probably going to have to
>>subclass objectgenerator for every database we support to deal with
>>their different personalitys. Since we currently only support MySQL,
>>this isnt a huge amount of work. However, going forward, the question of
>>access to every possible backend becomes an issue.
> 
> 
> I like the idea of adding in type checking or validation.  We could
> either make that a switch on the generator, or a run-time swtich so
> you could use it in development, and turn it off for production for
> speed/performance reasons.
> 
> 
>>What do you think? This lack of data checking in generated objects is
>>making me seriously rethink their merits. Sure it takes longer, but I
>>can write an object by hand that will not accept bad data. I'd rather
>>spend the time up front writing the object, than debugging on insert
>>errors.

very good idea.  =)  despite admittedly having only 2 brain cells, jason 
comes up with another good idea.  8^)

> I have a new proposal for the generator.  The current appraoch sucks.
> I don't think we need to sub-class the generator, we need to throw it away.  
> As It is currently written it is a bad approach (too much maintenence).
> 
> I propose that we do what Justin and I did for the last project at HMS:
> use PSP to generate the objects.  It's _soo_ much easier.  We write
> a driver program:
> 
>   dbRecGenerator.pl --dbiDsn="dsnString" --dbUser=user --dbPass=pass \
>      --dbTable=TABLE_NAME --genClassName=Some::Class::RecName \
>      --pspTemplate=recGen.psp
>      
> The driver program connects to the database, and puts togeather a data
> structure (or better yet, an object) that has all of the data about the
> table.  Then the PSP looks like this:
> 
>   <%
>     my $tableInfo = $self->query()->param('tableInfo');
>     my $packageName = $self->query()->param('packageName');
>   %>package <%= $packageName %>;
>   use strict;
>   use warnings;
>   <%
>     foreach my $use ( @someListOfObjectsToUse ) {
>   %>
>   use <%= $use %>;
>   <%
>     }
>   %>
>   our @ISA = qw( <%= @ourParentList %> );
> 
>   <% foreach my $colInfo ( $tableInfo->columns() ) { %>
>   sub <%= $colInfo->functionName() %>
>   {
>     my($self) = @_;
>     if( @_ > 2 ) { # a set
>       $self->validate<%= $colInfo->functionName() %>($_[1]);
>       return $self->setVal(x,y,z); # something like that
>     }
> 
>     # put get code here
>   }
> 
>   sub validate<%= $colInfo->functionName() %>
>   {
>     my($self,$val) = @_;
>     # do your validation based on the col type and precision here...
>   }
>   <% } %>
> 
>   sub recInsertSql
>   {
>     # return sql for a basic insert
>   }
>   # also do a select, update, and delete...
> 
>   1;
> 
> Does this make sense?  Then you can just substitute out the particular
> psp template that generates the recod objects in the way you like it to.
> 
> The templates will be infinitly easier to create and maintain than the
> current borked generator design.
> 
> Comments?

ah ha.  and now we come full circle to why i installed pas in the 1st 
place: to see about generating php record objects and to poke it where 
it hurts.  :0)  given that pseudo code, i'll hafta try it out.

sorry i didn't comment more, but we're putting a bid on yet another 
house and my PERL5LIB is acting up.  :-(

peace,

doug