Re: [svn:dbi] r14993 - dbi/trunk/ex

On Sun, 13 Nov 2011 10:42:34 +0000, Tim Bunce <[email protected]>,
"H.Merijn Brand" <[email protected]> wrote:

> On Thu, Nov 10, 2011 at 09:15:43AM -0800, [email protected] wrote:
> > +} elsif ($driver eq 'Unify') {
> > +    # Unify does not have varchar
> > +    $h->{ChopBlanks} = 1;
> 
> Why ChopBlanks? Worth a # comment.

I thought that the comment "Unify does not have varchar" was clear
enough. Unify only has "char", so when fetching a char(20) field where
only two characters were stored, you'll get it space-padded to 20 back.

> > +    $blob_column_type = 'binary';
> > +    $unicode_column_type = 'char';	# or text
> > +    $h->{uni_unicode} = 1;	# Available in the upcoming 0.81
> > +    $length_fn = 'undefined';	# I don't think Unify has a function like this
> 
> You can't ask Unify for the length of a string? Really?

I didn't find it in the docs. really

> >  sub do_connect {
> > -    my ($dsn, $user, $pass, %attr);
> > -    if (@ARGV) {
> > -        # eg unicode_test.pl "dbi:Pg(AutoCommit=0):host=example.com;port=6000;db=name" user pass
> > -        ($dsn, $user, $pass) = @ARGV;
> > -    }
> 
> Please restore that behaviour. Having people hard-code their own won't
> scale well.

That behavior is still valid if you call it like that

> > +    $user //= $ENV{DBI_USER} // undef;
> > +    $pass //= $ENV{DBI_PASS} // undef;
> 
> Please avoid //= etc. The test scripts should be runnable with older perls.

This script was for analysis only, not for inclusion in the test suite.

> > +    $h->commit if $driver eq 'Unify';
> 
> >      return lives_ok {
> > +        diag ($sql);
> >          my $s = $h->prepare($sql);
> >          $s->execute;
> > +	$dbd eq "DBD::Unify" and $h->commit;
> 
> Better as:
> 
>   +    $h->commit if $driver eq 'Unify';

in the eye of the beholder. I /did/ try to use the style used, but
personally I *never* use statement modifiers. It doesn't fit to my
mind. When taking over someone else's code, this is - next to layout
and indents - the first thing to correct.

to me

   expression and action;

reads MUCH better than

   action if expression;

> for consistency with the others.

that's what I said. Sorry i missed that. I tried.

> Thanks!
> 
> Tim.


-- 
H.Merijn Brand  http://tux.nl      Perl Monger  http://amsterdam.pm.org/
using 5.00307 through 5.14 and porting perl5.15.x on HP-UX 10.20, 11.00,
11.11, 11.23 and 11.31, OpenSuSE 10.1, 11.0 .. 11.4 and AIX 5.2 and 5.3.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/