Re: Can I from within a module access variables from the calling program? (modular design)

[tye]

Wow. Those are some really horrible suggestions for how to solve this problem.

A well-designed component gets information from the caller by the caller passing in the information to the component. Having components try to reach up into the parent and pull stuff out of the parent is a much worse design choice than even just the using of a global variable.

You usually pass data into a module either when you 'use' the module or by calling subroutines or methods from the module. For example:

use Your::Module( $log_file );

or

use Your::Module;
Your::Module->set_log_file( $log_file );

The first example leads to some subtle gotchas as well, so just use the second method (just like the designers of Log::Log4perl did):

package Your::Module;
# ...
sub set_log_file {
    my( $we, $log_file ) = @_;
    Log::Log4perl->init( \"... $log_file ..." );
}

You don't see Log::Log4perl telling you, "Just set the $log_conf global variable in your package to contain your configuration string and Log::Log4perl will go pull the value from it".

Having the child module have to know the name of the variable and forcing the parent to make that variable global are both design mistakes. Using PadWalker is particularly crazy. Can you imagine Log::Log4perl saying further "Note that if you declare the variable to be hidden from other modules by using 'my', Log::Log4perl will thwart your stated goal of hiding the variable and will dive into the guts of Perl to find the variable anyway so you might want to add a comment to make this clearer: my $log_conf # Not really private!".

Worse than the "we'll pass the information by both sides happening to use the identical name" part of this is that you also put a subtle dependency on the order in which things get done. Consider this example:

package main;
use strict;
use Log::Log4perl;
use Your::Module;

our $log_file = ...;
Log::Log4perl->init( ... );

and

package Your::Module;

Log::Log4perl->init( \qq< ... $main::log_file ... > );

So, after you notice that this isn't working, you go to fix the order in which things are done using a more awkward layout:

package main;
use strict;
use Log::Log4perl;

our $log_file = ...;
Log::Log4perl->init( ... );

use Your::Module; # Must happen after $log_file is set. DON'T MOVE UP!

But that doesn't work either.

Pass in the data needed by code when you call the code that needs the data. Then the audience playing at home has a better chance to more quickly understand what is going on.

But I also thought that Log::Log4perl dislikes being initialized more than once in the same process. So I suspect that your design has even more problems in it than just the passing of the log file name. Indeed, seeing you repeat the identical configuration for Log4perl in two places looks like the sign of poor design to me. It sounds like you noticed that you were computing the name of the log file in two places only because you didn't manage to always come up with the same name in both places.

Computing the same thing in two places using code that was copy-and-pasted is poor design. Copying and pasting code is always a bad idea (usually a really bad idea). But declaring it unacceptable for the two sides to come up with different answers is even worse.

Repeating the configuration and initialization of Log::Log4perl looks like a design mistake. Of course, I find the initialization of Log::Log4perl to be terribly awkward. The way one uses Log::Log4perl is moderately awkward, something that I'd probably consider a good choice in most modules but one that I find unfortunate for a logging module.

I believe that a logging module should go to unusual lengths to make it extra easy to add logging so that more logging is likely to get added before the need for the logging actually comes up. When a problem is found with some code and I need to debug what is going on in a deployable service, then it is often inconvenient to reproduce the problem in a way where I can use the Perl debugger. So being able to just turn up some existing logging and get a lot more details about what is going on is often a huge time saver.

So Log::Lager tries to make it really easy to add some trace. The module needs further enhancements (some of which are happening now, at least in the git repo, soon to be on CPAN, I hope). So you might also look into using it.

[HJO]

Waow...

first of all thank you for giving me the "right" way to do this...

But even after reading it, I still don't understand everything you're saying... I'll wait tomorrow to read it with my mind rested so I can apply what you're saying ^^"

[HJO]

Hi tye,

I finally found some time to try to understand your anwser, I think I did, and most of what you are saying makes sense to me.

So first, I'm using Log::Log4perl because when I looked at a module that could log things, it was one of the first to pop out, and with sufficient help on the web and good comments on it so I could easily learn to use it. (BTW I checked your Log::Lager, and didn't found it on CPAN as your link indicates, and on google I found it on v 0.04.XXX, so thanks for the alternative, but I will stick on Log::Log4perl a little bit longer...)

Secondly, yes my design is very poor, because I don't plan very much things when I'm coding, due to the fact that I'm quite a beginner (well not a complete one) on Perl and on developping script in a professionnal environment... So yes, my code is not very beautiful on those aspects, but I'm trying with the best I can do, but of course, it's not always (well I'd even say never) in the way of Perl, or the good practices with module programming...

So my original issue was : how can I have some coherent logging from my script AND my module (in the same file), without having to double the declaration of my logger ? (of course it would also be perfect if this solution was compatible with Log::Log4perl)

It seems you're quite knowledgeable on this subject, so I'm asking for help and guidance.

Thank you very much in advance for your answer.