in reply to passing subroutine args as a hash: why not?

I'm wondering why more people don't call subroutines with a single hash as the argument.

Most people don't write subroutines that take 61 arguments! :-) I've gotta wonder what that subroutine is doing, and whether it may be better to break it up into several subroutines that each do something small and specific.

That said, when you do have a fairly large number of subroutine arguments, I think passing them as named parameters in a hash is a fine idea, for the reasons you mentioned.

-- Mike

--
just,my${.02}

  • Comment on Re: passing subroutine args as a hash: why not?

Replies are listed 'Best First'.
Re: Re: passing subroutine args as a hash: why not?
by hardburn (Abbot) on Jun 05, 2003 at 18:23 UTC

    For your typical sub, there is a great problem with that many params. However, I can see how a perfectly good object would have a constructor with quite a lot of args. Perhaps not 61, but they often have a lot more than a typical sub (just look at all the options in the HTML::Template constructor).

    ----
    I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
    -- Schemer

    Note: All code is untested, unless otherwise stated

      Actually, I'd say the paucity of arguments is just as important in constructors as in subroutines and for reasons beyond the mere fact that in Perl a constructor just is a class method. For one thing, any object that takes more than a couple of seconds of consideration as to how it may be instantiated will discourage its own use. In either case it indicates generally that insufficient thought was given to breaking up responsibilies among constituent parts. Not always, but often the first thing to consider if you've got vast numbers of arguments to a constructor is the use of inheritance to encapsulate the differences among data and behavior normally conveyed via parameters.

      Consider, say, a Logger class with multiple different types of logging available, e.g., types of warning raised if any, location of error logs, email or page contacts, etc. Better to create a CarpLogger subclass that inherits behavior and already knows what to do than to clutter Logger with if/else code deciding actions based upon arguments. A smaller, simpler constructor improves maintainability and usability.


      "The dead do not recognize context" -- Kai, Lexx

        Looking back at HTML::Template, I count a total of 31 params passed to its constructor. Granted, some of them would never be used together (such as arrayref and filename), but it will accept that many. Despite this fact, the module doesn't strike me as having a bad design. Most of those params would need to be passed to a subclass anyway, and the few params left over (perhaps some of the caching options, strict, or die_on_bad_params) just don't make it worth the trouble of inheritance (IMHO).

        Further, inheritance is a major kludge in Perl5 (kludgier than the rest of its object system), and should generally be avoided if possible.

        ----
        I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
        -- Schemer

        Note: All code is untested, unless otherwise stated

Re: Re: passing subroutine args as a hash: why not?
by Willard B. Trophy (Hermit) on Jun 05, 2003 at 18:58 UTC
    The 61-headed 350+ line monster sub is taking all the registration information from a huge web form, and storing it in several tables in a database. I think it's all lumped together so the db can be rolled back if something goes wrong.

    I wouldn't have done it that way had it been my code. But I'm just the maintainer, and I have to keep my new named-parameter routine in line with the old one until we're sure everything works.

    --
    bowling trophy thieves, die!

      Why aren't those 61 arguments encapsulated in an easy to use data structure? If they were, your question would be moot. Consider how much improved your life would be if you changed the code so that it things looked like this:

      my $query = CGI->new; my %reg_info = parse_registration_info($query); my $success = store_registration_info(\%reg_info, $database_handle);
      or, alternatively, like this:
      my $query = CGI->new; my $registration = OurWebsite::User::Registration->new($query); my $success = $registration->store($database_handle);
      Sure, that's contrived. But the point is that passing around dozens of arguments is ridiculous. If you changed the function to take named arguments, you'd be passing twice as many arguments total (the args and their names.) Working with that much cruft is neither easy nor necessary. Build a data structure that holds that information once, as you parse it, and then pass around a single reference to that structure.

      -sauoq
      "My two cents aren't worth a dime.";
      
        They're not wrapped in a data structure, 'cos that's the way the code is, and modifications are needed by tomorrow.

        --
        bowling trophy thieves, die!