use Perl

• Some nits to pick (Score:2)

  by autarch (914) on 2007.09.07 0:14 (#57570) Homepage Journal

  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" 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.
  
  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.
  

  Reply to This

  • Re: (Score:1)

    by fireartist (6128)

    I got the impression that all CPAN modules should use the logging interface, but not set any logger themselves.
    So the only place the logger would be set would be in your 'top level' application, or framework (catalyst, jifty, etc).
    • Re: (Score:1)

      by jonswar (7880)

      That's exactly right. Since the setting of Log::Abstract is (by design) global across the application, it would be considered bad behavior for an arbitrary CPAN module to muck with it, in the same way it would be bad behavior to unset $^W, or clear @ARGV, or read from STDIN - unless that was part of the proscribed behavior of the module.
      
  • Re: (Score:1)

    by jonswar (7880)

    > 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 eith
    • 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.