use Perl

• Some nits to pick (Score:2)

  by autarch (914)

  Generally, I think this is a good idea, but I have some nits to pick.
  
  First, the set_logger() method is not actually setting a logger at all, which is confusing. It's setting a callback that returns a logger, so maybe it should be set_logger_factory() or something like that. This is particularly problematic since there's a get_logger() method, which doesn't return the thing passed to set_logger()!
  
  I'm not sure that passing in a sub ref is the best API either. My gut feeling is that "the average programmer" is
  • Re:Some nits to pick (Score:1)

    by jonswar (7880) <swartz@pobox.com> on 2007.09.07 13:34 (#57603) Journal

    > First, the set_logger() method is not actually setting a logger at all, which is
    > confusing. It's setting a callback that returns a logger, so maybe it should be
    > set_logger_factory() or something like that. This is particularly problematic since
    > there's a get_logger() method, which doesn't return the thing passed to set_logger()!
    
    You're right, set_logger() should be renamed, though I'm not sure I can bear to use "factory". :)
    
    >
    > I'm not sure that passing in a sub ref is the best API either. My gut feeling is that "the
    > average programmer" isn't really comfortable with subroutine references. OTOH, I see the
    > need for being able to pass in a log making strategy, as opposed to a single log object.
    >
    > Maybe there could be a super-simple API where you pass in a log object, and a more complex
    > API where you pass in a sub ref or logger factory class.
    
    Unfortunately, a super-simple API wouldn't work for log4perl.
    
    Perhaps set_logger_factory() or whatever it is ultimately called should be the low-level interface, and logging frameworks can provide higher-level mixins to make things easy for the average user. e.g.
    
          use Log::Dispatch; # this also defines Log::Abstract::use_log_dispatch
          my $dispatcher = Log::Dispatch::File->new(...);
          Log::Abstract->use_log_dispatch($dispatcher);
    
          use Log::Log4perl; # this also defines Log::Abstract::use_log4perl
          Log::Abstract->use_log4perl(); # no argument needed
    
    >
    > I also wonder how this package-level globalness will work. What if I have a CPAN module
    > that wants to set the Log::Any logger, and I want to use that module in my app but set a
    > different logger? Is it last set wins?
    >
    > Imagine the problems this could cause under mod_perl. Maybe the answer for mod_perl is
    > that your app needs to set the logger on every request, not just once at startup time.
    
    I just think we have to trust modules not to muck with global state that they aren't meant to muck with. mod_perl and other persistent environments depend on this in a variety of ways.
    
    And yes, I think the last set has to win, to allow applications to modify the setting in mid-stream.
    
    Jon
    
    

    Reply to This

    Parent

    • Re: (Score:1)

      by Aristotle (5147)

      You’re right, set_logger() should be renamed, though I’m not sure I can bear to use “factory”. :)

      The name you are looking for is register_logger.