Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

Re: Code cleanup; how best to deal with: defined(%hash) is deprecated at... (defined--)

by tye (Sage)
on May 22, 2014 at 06:33 UTC ( [id://1087068]=note: print w/replies, xml ) Need Help??


in reply to Code cleanup; how best to deal with: defined(%hash) is deprecated at...

After reading a ton of Perl code (much of it written by co-workers), I decided that the use of defined and exists can often be nearly random. Much less frequently but much more seriously, I've seen several cases of failures (some of them even in Production) caused by code like:

$foo->{gnarfle} = 'garthok' if defined $foo;

So I've instituted some simple best practices:

  • Don't use defined (nor // nor //=) unless undef has a useful meaning (distinct from "just false").
  • Don't use exists unless "missing", "undefined", and "false" all have distinct meanings (which is usually a bad idea).
  • If 0 is a useful value and not "the default", then use a boolean test against length. (This ends up being pretty rare, though.)
  • Otherwise (the large majority of the time), just use a simple boolean test (or || or ||=).

In particular, there is no such thing as a useful, false reference (unless you are overloading in a way that could return a false value -- and overloading is very rare in our code). So don't use defined (nor exists nor // nor //=) with references.

Somewhat related, for string-valued database columns, if the empty string doesn't have special meaning, then declare the column "not null default ''", so you don't have to deal with a random mix of NULL and '' values.

Similar to not using exists, don't check for missing positional parameters by checking the value of 0+@_ unless "missing", "undef", and "false" all have distinct meanings (which is usually a bad idea).

- tye        

  • Comment on Re: Code cleanup; how best to deal with: defined(%hash) is deprecated at... (defined--)
  • Select or Download Code

Replies are listed 'Best First'.
Re^2: Code cleanup; how best to deal with: defined(%hash) is deprecated at... (defined--)
by sundialsvc4 (Abbot) on May 22, 2014 at 11:01 UTC

    The only comment here that I don’t strongly-agree with, is the one about a database-column being not null default ''.   Although it is a bit of a pain to deal with the difference in code, there is nonetheless an important difference in meaning.   The absence of any value is not the same as an empty-string value.   The latter, to me, implies that “the value is known, and it is known to be an empty-string.”   Queries that might be run by clients entirely separate from this application, e.g. SELECT COUNT(colname) issued by some report-whatnot, would be thrown-off considerably by empty-string values, being lured into counting something that more-properly “isn’t there.”

    Mind you, it is not that I categorically-disagree with this practice; merely that I often-do.

    Beyond that, a series of very excellent points; upvoted.

      You probably missed the if the empty string doesn't have special meaning part that describes, one might think that a bit more laconically than needed, exactly the conditions you mentioned in your post.
      لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ
      Howdy!

      Some databases consider the empty string to be NULL. I'm looking at you, Oracle. Last time I looked, that execrable behavior is contrary to the SQL standard.

      yours,
      Michael

      From a relational database theorists' perspective, I would have preferred to declare "not '' default null" but that isn't supported so simply or easily.

      From a practical perspective, I actually ended up preferring the route we went.

      A quick aside: The behavior of sum() and avg() when given values that are null makes perfect sense. But I think count() would have been better implemented as count(BOOL) so one would write things like count(foo <> '') or count(foo is null). Then avg(foo) would be roughly sum(foo)/count(foo is not null). I find the behavior of count(blah) to fairly often be a source of mistakes. And I also find the desire for "count how many times this isn't null" to be pretty rare in practice.

      So I was happy to avoid (the more frequent but still somewhat rare but also more problematic, IME) problems like "where foo <> 'bar'" skipping null values.

      - tye        

Re^2: Code cleanup; how best to deal with: defined(%hash) is deprecated at... (defined--)
by taint (Chaplain) on May 23, 2014 at 07:47 UTC
    Hello, tye, and thank you for some damn good rules to code by.

    With the possible exception, as noted by sundialsvc4, It's all hard to argue with.

    To tell you the truth. I had set out to quickly get all the bits and pieces together, so I could have it working, and simply refine it from there. But quite some time later, and a boat load of additional features I hadn't initially imagined. It's ultimately going to need a bit of an overhaul. Hind sight being 20-20. I really should have let the whole thing "stew" in my head for a great deal longer. Giving me much greater insight, on how I should really have approached the whole thing, to begin with.
    But I was really motivated, and actually had some time to work on it. So I seized it, and ran with it.

    But in my humble defense, it was long ago, and alot has changed since then. Not the least of which, is my understanding of the language, and I'd like to think, a better, more "educated" approach to accomplishing such things. :)

    Thank you very much, for taking the time to share some advise, tye.

    --Chris

    ¡λɐp ʇɑəɹ⅁ ɐ əʌɐɥ puɐ ʻꜱdləɥ ꜱᴉɥʇ ədoH

      So how are you going to deal with the extension problem? Or are you going to allow 3rd-party extensions? (I would bet that many of us here have thought about how to write the Drupal-killer in Perl but have smacked our heads against that wall.)

      It helps to remember that the primary goal is to drain the swamp even when you are hip-deep in alligators.

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others admiring the Monastery: (3)
As of 2024-11-05 04:30 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    chatterbot is...






    Results (27 votes). Check out past polls.