Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW
 
PerlMonks  

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

by taint (Chaplain)
on May 22, 2014 at 03:42 UTC ( [id://1087048]=perlquestion: print w/replies, xml ) Need Help??

taint has asked for the wisdom of the Perl Monks concerning the following question:

Greetings, Monks. I've been cobbling away at creating a Perl based CMS, for a few years now. It's turned out quite nice, and is very featurefull -- including a Forum.

To the point;
As I indicated, it's been built over several years, and it shows. So I'm hoping to clean things up enough to release it into the wild.

I'm currently looking at the following message from perl:

defined(%hash) is deprecated at
This, as many/most of you already know, was depreciated around v.5.10. The actual code block(s) that trigger the message are:
... if(($position eq 'l' || $position eq 'r') && defined %leftright) { %tags = (%tags,%leftright); } elsif(($position eq 't' || $position eq 'b') && defined %topbott +om) { %tags = (%tags,%topbottom); } ...
While Perl, itself, suggest simply removing defined, and I suppose I have little reason to doubt the brilliance of Perl -- hell, it's smarter than I am. But my investigations indicate that others looking to deal with this same situation, do so in a myriad of different ways. So I thought it prudent to get some feedback from those here, I've come to trust. :)

In other words; How would you deal with it?

Thanks, for all your time, and consideration.

--Chris

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

Replies are listed 'Best First'.
Re: Code cleanup; how best to deal with: defined(%hash) is deprecated at...
by davido (Cardinal) on May 22, 2014 at 03:55 UTC

    Add "use diagnostics;" to the top of your script and run it again. You'll see a more thorough explanation, including this:

    Although defined %hash is false on a plain not-yet-used hash, it becomes true in several non-obvious circumstances, including iterators, weak references, stash names, even remaining true after undef %hash. These things make defined %hash fairly useless in practice. If a check for non-empty is what you wanted then just put it in boolean context (see "Scalar values" in perldata):

    if (%hash) { # not empty }

    There's not much I can add to that, other than to say that by following the advice above you may actually be removing an undiscovered bug.


    Dave

      Thanks davido.

      Darn good advice, and so obvious too. :) I hate it when I overlook the obvious. It's so embarrassing. :P

      I'll investigate with diagnostics.

      Actually. They're pretty much true|false booleans. That I use to determine the chosen positions of the "blocks" in the layout. So I'm fairly confident that the whole thing won't explode, or anything. But just the same; thought it worth asking. Before a final commitment.

      Thanks again, davido. For taking the time to reply

      --Chris

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

        Try to remove defined and run your tests. If everything passes, you are probably safe.
        لսႽ† ᥲᥒ⚪⟊Ⴙᘓᖇ Ꮅᘓᖇ⎱ Ⴙᥲ𝇋ƙᘓᖇ

        The only final commitment ought to be $ git merge bool_hash. Until then it's pretty easy to git branch -D bool_hash

        Even if you've merged, you can still rewind without causing grief, as long as you haven't pushed somewhere where others might suffer. I know not everyone's sold on version control tools, but I find them empowering; I can try things without having to remember every change I made along the way. And if a plan comes together, I can merge. If not, cut off the experimental branch with the confidence of knowing that the stable branch is still untouched.

        If your project hasn't been using VC, it's not too late to start. Assuming it's currently in a working state, initialize a repo, create a "refactoring" branch (probably use a more descriptive name), and get to work. At each major decision, branch, then if it works well, merge.


        Dave

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

    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        

      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        

      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.
Re: Code cleanup; how best to deal with: defined(%hash) is deprecated at...
by NetWallah (Canon) on May 22, 2014 at 05:29 UTC
    Here is an more declarative alternative to do the tests and assignment you show above:
    %tags = ( %tags, %{ {l=>\%leftright, r=>\%leftright, t=>\%topbottom, b=>\% +topbottom }->{$position} } );
    It will give the right results (with warnings) if any of the expected values is not defined, or incorrect.

            What is the sound of Perl? Is it not the sound of a wall that people have stopped banging their heads against?
                  -Larry Wall, 1992

      Hello, NetWallah.

      Thank you very much for taking the time to do that for me. I like it. You're right, and it's alot smaller too. :). I'm afraid I sometimes get so focused, and obsessed on what I'm doing. It's kind of a "Forrest for the Trees" sort of thing. I really need to condition myself to step back, or away from it, more often. :)

      I really appreciate your taking the time to whip that up, for me, NetWallah.

      All the best.

      --Chris

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

Re: Code cleanup; how best to deal with: defined(%hash) is deprecated at...
by boftx (Deacon) on May 22, 2014 at 05:04 UTC

    I agree with davido above. The only thing I would say is that I find myself rarely using my %hash; anymore, preferring my $hashref = {}; instead. It doesn't directly help with the example problem given, but might help elsewhere if you are passing the data around a lot.

    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: perlquestion [id://1087048]
Approved by davido
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others contemplating the Monastery: (5)
As of 2024-03-29 14:16 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found