Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 556 forks source link

some documented practices in CORE and ways they might be improved #14767

Closed p5pRT closed 9 years ago

p5pRT commented 9 years ago

Migrated from rt.perl.org#125443 (status was 'resolved')

Searchable as RT125443$

p5pRT commented 9 years ago

From perl-diddler@tlinx.org

Created by perl-diddler@tlinx.org

Was looking at the 5.20.2 docs and noticed a few places you could make things a bit more tidy and less cluttered..

(I.)Universal man page has​:

  The problem is that this code will never call an overridden "isa"   method in any class. Instead\, use "reftype" from Scalar​::Util for the   first case​:   use Scalar​::Util 'reftype';   $yes = reftype( $h ) eq "HASH";

Instead of the above\, how about​:

  use Types​::Core;   $yes = HASH $h;

You could use​:   use Types​::Core(typ);   $yes = typ $h eq HASH

But I think the 1st one is simpler -- what do you think? I admit to prejudice\, but 'typ' seems less likely to cause confusion than something that has 'ref' in it -- so reference & type -- seemed of parallel usefulness to use 1st 3 letters for each (but that's a minor and subjective 'nit').

II) in perlobj there is this text​:   use Scalar​::Util 'blessed';

  my $foo = {};   my $bar = $foo;

  bless $foo\, 'Class';   print blessed( $bar ); # prints "Class"

  $bar = "some other value";   print "bar blessed​:".blessed( $bar ); # prints undef

1) first problem\, I tried the above and it prints *nothing* unless you have warnings turned on -- and then you get "Use of uninitialized value in print at - line \".

If you want it to print *something* (**anything!**​: you don't even get "bar blessed​: \)\, you could use 'P'\, which not only prints 'undef'\, the rest of the output\, but adds a '\n' at the end I don't think there is anyway to get your 'undef' and your line output w/o explicit tests for 'undef' -- which get rather cumbersome in code.

  use P; use Types​::Core q(blessed);   my $bar = "some other value";   P "bar blessed​: %s"\, blessed $bar;'

  bar blessed​: ∄

the symbol '∄'\, means "[for all things\,] there does not exist" (closest I could find to "undef" in unicode).

A bit later (same manpage/pod) there is the text​:

III) If you want to know whether a particular scalar refers to an object\,   you can use the "blessed" function exported by Scalar​::Util\, which is   shipped with the Perl core.

  use Scalar​::Util 'blessed';

  if ( defined blessed($thing) ) { ... }

  ...

IV) Note that "blessed($thing)" will also return false if $thing has been   blessed into a class named "0". This is a possible\, but quite   pathological. Don't create a class named "0" unless you know what   you're doing.   ...

V) If you simply want to check that a variable contains an object   reference\, we recommend that you use "defined blessed($object)"\, since   "ref" returns true values for all references\, not just objects. -----

Couple things here​:

1) a simpler solution (eliminate the 'defined')​:

  use Types​::Core q(blessed);   bless my $thing={}\,"BLESSME";   if (blessed $thing) {...}

In reference to "(IV)" --

  bless my thing={}\,0;   P "thing=%s"\, blessed $thing ? "blessed" : "not";

  prints "not" for 0 or '0'.

In all fairness the same happens if you use Scalar​::Utils 'blessed'. Why? You can't have a class starting with a number\, that I'm aware of -- so -- I would strongly agree to not create those '0' classes unless you are using some new experimental feature. In most cases\, I can't think of a case where the defined is necessary.

In reference to '(V)'...wouldn't it be better to check if the 'ref' not only is 'blessed'\, but is also of the *type* you want? I.e. if you do this​:

  bless my $object = []\, "myobj";   if (defined blessed($object)) {   $object->{mystuff}={...};   }

You get​: "Not a HASH reference at -e line xx". As a coding practice\, that's really not much better than using 'ref'. To me\, if you want to protect your code\, you need to check the 'blessed'ness and the 'typ'e​:

  use Types​::Core q(blessed);   bless my $object = []\, "myobj";   die "oh woe is me" unless blessed HASH $object;   $object->{mystuff}={...};'

  oh woe is me at -e line 3.

But if we check that we have a blessed object in the form we "expect" (or want/need for our purposes)​:

  bless my $object = {}\, "myobj";   $object->{mystuff}={...} unless blessed HASH $object;'

  the blessed HASH checks for all we need and 'stacks' for increased legibility..

  One last thing I came across recently​:

VI -- on the 'perlref(1)' manpage\, it cautions against accidently autovivifying things​:

  use P; use Types​::Core;   my @​array;   $array[23]->{"foo"}->[0] = "January";   P "array=%s"\, \@​array;' array=[∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, ∄\, {foo=>["January"]}]

That is definitely auto-viv'ed\, It's not the easiest\, but one could use a function in 'Types​::Core'\, called 'EhV' (Exists hash Value)​: (if you think there is any function in perl core or cpan that can show the effect of autovivification as easily as using P\, I'd love to know about it. P's output of \@​array\, was meant to take little space\, but show what it was asked for\, so it's not as pretty like Data​::Dumper\, but it concisely shows the effects of autovivification in a way few other (if any) single functions could.

  use P; use Types​::Core;   my @​array;   my $r= EhV \@​array\, 23 && EhV $array[23]\, "foo" && EhV $array[12]{foo}\, 0;   if ($r) {   $array[123]->{"foo"}->[0] = "January";   }   P "array=%s"\, \@​array;'   array=[];

In this case we avoided the autoviv -- the testing for which can be a chore especially if you check for 'definedness' and refs of the appropriate type at each step. (No example from me... too much work).

  Anyway\, FWIW Types​::Core will use 'Scalar​::Utils for it's 'blessed' function under conditions defined in Type​::Core's manpage​:

  blessed REF; #is REF blessed or not?

  Included for it's usefulness in type checking. Similar   functionality as implemented in Scalar​::Util (uses "Scalar​::Util"   if available\, though it is not needed).  
'typ' doesn't use 'reftype' as 'reftype' wasn't around when it was developed (maybe I should update Types​::Core -- since I'm sure the Scalar​::Utils implementation is faster than my perl-only method.

Some of the above (output of undef\, packages named 0) really should be corrected. The rest of the suggestions are pretty much just that...

Cheers!

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.20.2: Configured by law at Mon May 25 23:15:24 PDT 2015. Summary of my perl5 (revision 5 version 20 subversion 2) configuration: Platform: osname=linux, osvers=4.0.4-isht-van, archname=x86_64-linux-thread-multi-ld uname='linux ishtar 4.0.4-isht-van #2 smp preempt sat may 23 21:04:29 pdt 2015 x86_64 x86_64 x86_64 gnulinux ' config_args='' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=define, uselongdouble=define usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector' ccversion='', gccversion='4.8.3 20140627 [gcc-4_8-branch revision 212064]', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t', lseeksize=8 alignbytes=dddddddddd, prototype=define Linker and Libraries: ld='gcc', ldflags =' -fstack-protector' libpth=/usr/lib64/gcc/x86_64-suse-linux/4.8/include-fixed /usr/lib64/gcc/x86_64-suse-linux/4.8/../../../../x86_64-suse-linux/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=libc-2.19.so, so=so, useshrplib=true, libperl=libperl-5.20.2.so gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/home/perl/perl-5.20/lib/5.20/x86_64-linux-thread-multi-ld/CORE' cccdlflags='-fPIC', lddlflags='-shared -O2 -fstack-protector' @INC for perl 5.20.2: /home/law/bin/lib /home/perl/perl-5.20/lib/site_perl/5.20/x86_64-linux-thread-multi-ld /home/perl/perl-5.20/lib/site_perl/5.20 /home/perl/perl-5.20/lib/5.20/x86_64-linux-thread-multi-ld /home/perl/perl-5.20/lib/5.20 . Environment for perl 5.20.2: HOME=/home/law LANG=en_US.utf8 LANGUAGE (unset) LC_COLLATE=C LC_CTYPE=en_US.UTF-8 LD_LIBRARY_PATH=/libdl LOGDIR (unset) PATH=/home/perl/perl-5.20/bin:/sbin:.::/home/law/bin:/usr/local/bin:/usr/bin:/bin:/opt/kde3/bin:/usr/lib/mit/bin:/usr/lib/mit/sbin:/usr/lib/qt3/bin:/usr/sbin:/etc/local/func_lib:/home/law/lib:/home/law/bin/lib PERL5OPT=-Mutf8 -CSA -I/home/law/bin/lib PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 9 years ago

From zefram@fysh.org

Linda Walsh wrote​:

    $yes = reftype\( $h \) eq "HASH";

Instead of the above\, how about​:

           use Types​::Core;
           $yes =  HASH $h;

Absolutely not. We do not want to encourage the use of such a poor-quality module. It being non-core also counts against it.

1) first problem\, I tried the above and it prints *nothing* unless you have warnings turned on -- and then you get "Use of uninitialized value in print at - line \".

The phrase "prints undef" is a little misleading\, sure. We could usefully add a "// 'undef'" to the code.

If you want it to print *something* (**anything!**​: you don't even get "bar blessed​: \)\,

"bar blessed​:" does get printed. Maybe you lost it due to lack of trailing newline​: the code is somewhat relying on $\ having been set (for example by -l on the command line).

                               you could use 'P'\,

No way.

1) a simpler solution (eliminate the 'defined')​:

use Types​::Core q(blessed);

We still don't want to use that module. But looking at that function more specifically\, it has more gotchas than Scalar​::Util​::blessed(). It will return false for something blessed into a class whose name matches the name of a built-in type such as "ARRAY". Also\, in place of a true value it returns the reference that was passed in\, which can then behave as false depending on overloading. (NB​: this perlbug RT ticket is not the place for protracted discussion of the flaws of Types​::Core.)

 You can't have a class starting with a number\, that I'm

aware of

bless() accepts any string as the class name parameter.

In reference to '(V)'...wouldn't it be better to check if the 'ref' not only is 'blessed'\, but is also of the *type* you want?

That paragraph isn't concerned with performing operations directly through the referenece. It's about distinguishing blessed objects from unblessed things; the physical type of the blessed object is irrelevant. Indeed\, if one wanted to perform direct operations on the object internals\, checking that it is blessed into the correct class would be necessary\, rather than merely checking whether it is blessed. Once it is found to be blessed into the correct class\, checking the physical type is rather redundant.

-zefram

p5pRT commented 9 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 9 years ago

@rjbs - Status changed from 'open' to 'resolved'

p5pRT commented 9 years ago

From @rjbs

* Linda Walsh \perlbug\-followup@​perl\.org [2015-06-20T23​:20​:41]

Instead of the above\, how about​:

            use Types​::Core;
            $yes =  HASH $h;

You could use​: use Types​::Core(typ); $yes = typ $h eq HASH

But I think the 1st one is simpler -- what do you think?

Not a fan. It makes code backward incompatible with basically no gain\, and conflates type with reftype\, which are distinct things. (The type of $h is ref\, and the type of the thing referenced is hash.)

II) in perlobj there is this text​: use Scalar​::Util 'blessed';

     my $foo = \{\};
     my $bar = $foo;

     bless $foo\, 'Class';
     print blessed\( $bar \);      \# prints "Class"

     $bar = "some other value";
     print "bar blessed​:"\.blessed\( $bar \);      \# prints undef

1) first problem\, I tried the above and it prints *nothing* unless you have warnings turned on -- and then you get "Use of uninitialized value in print at - line \".

Thanks\, this can be improved. I've pushed a change to always print a visible string.

As for everything else on this point\, we already have a pretty printer in core\, and many more on the CPAN\, including P. I don't think there's call for a new one in core\, since core is already enough to bootstrap whichever printer you like.

In all fairness the same happens if you use Scalar​::Utils 'blessed'. Why? You can't have a class starting with a number\, that I'm aware of -- so -- I would strongly agree to not create those '0' classes unless you are using some new experimental feature. In most cases\, I can't think of a case where the defined is necessary.

Crazy code has used 0 as a package name. It works\, although you need to muck about with the symbol table directly. In the future\, perhaps we can deprecate false package names\, but even then any code that runs where a ref can be blessed into 0 needs the defined.

It's obnoxious\, I agree\, but it's not wrong.

In reference to '(V)'...wouldn't it be better to check if the 'ref' not only is 'blessed'\, but is also of the *type* you want? I.e. if you do this​:

bless my $object = \[\]\, "myobj";
if \(defined blessed\($object\)\) \{
    $object\->\{mystuff\}=\{\.\.\.\};
\}

No. You shouldn't be doing ->{mystuff} unless the class has documented that as part of its interface.

-- rjbs