Re: [boost] [Review] The review of the Boost.Logging library starts today, November 7th

From: Andrew Schweitzer (a.schweitzer.grps_at_[hidden])
Date: 2005-11-13 13:23:55

-----------------------------------------------------------------
 >> - What is your evaluation of the design?

Good foundation, lots of features, but a bit complex and opaque.

Some main thoughts:
1) It might be helpful if loggers and managers were "promoted" to 
objects with an obvious interface that can be easily passed around.

1.1) I haven't had a problem passing loggers around by use using a 
logger& or a logger*, but the interface seems to assume that generally 
the code will be compiled where the log name is known. Most of the code 
I work on is header or dll code that needs to be passed a logger by the 
main system.

1.2) I think the whole design would be easier to "drop in" if it were 
provided with a "public" manager interface and the manager objects could 
be easily passed around.

1.2.1) I think the static manager makes the design harder to use and 
control in a system that isn't a single executeable. This could be 
solved by either forcing the user to declare the manager, or possibly by 
leaving it static, but making all the functions take an explicit manager 
ref or pointer, or making the functions members of the manager.

1.2.1.1) The static manager seems particularly problematic for dlls.

1.2.1.1.1) Currently, the dll can't call the api functions and act on 
the application's logs, since the calls will be compiled to act on the 
dll's static log manager (I think). Although "externally visible" is a 
design premise of the library, I don't think the dll can even enable or 
disable application logs it knows the name for because the call will go 
to its own logs of the same name. Similarly the application can't 
control the dlls logs for the same reason... I think!... maybe I missed 
how to do this. If that's the case maybe the doc just needs to be more 
extensive.

1.2.1.1.2) It looks like there could actually be multi-threaded issues 
writing to logs from a dll, because, if the log thread is used, both the 
dll and the app will start their own trhead to write to the log. 
Presumably the log is ultimately a single object, like the screen or a 
file. I suppose if the locking uses a "named" lock that the OS api can 
look up this could be solved. However, it looks like on windows at least 
a Critical Section is used, which I don't think is named, so I don't 
think the one or more dlls running the thread and the application would 
actually lock each other. It seems like they could step on each other.

1.3) I can't seem to pass around log levels. I think this is because 
they are const ints, not members of an enum. This means the following 
code won't compile:

void SomeClass::Dump(boost::logging::level_type eLevel,
                     boost::logging::logger& rLog)
{
        BOOST_SCOPEDLOGL(rLog, eLevel)
                        << "Some Msg" << m_iSomeMemberVariable << endl;
}

This is kind of important in our code.

2) BOOST_LOG and BOOST_SCOPEDLOG are way too many uppercase characters 
to type. :)
Ok, so that one's kind of petty. But seriously, as a veteran of many 
many many printfs, my wrists cringe when my brain tells them to type 
that out. Seriously again, I'm not sure this is an easy problem to 
solve. It's hard to come up with a meaningful short name that doens't 
clash with everything else. The best solution might be for the 
programmer to make an macro in their IDE that print "BOOST_LOG() <<" 
when they press ctrl+alt+b or something. If the name clash issue could 
be solved the boost log libary could provide some reasonable default. I 
added the following macros to my code, which I like, but I'm not sure 
are fit for a library:
#define LG(log) BOOST_LOG(log)
#define LGPTR(pLog) BOOST_SCOPEDLOG(*pLog)
#define LGM() LGPTR(m_pLog)	  //Log to member
#define LGP() if(pLog)LGPTR(pLog) //Log to param
Then this code works:
void SomeFunction(logger* pLog) {
        LGP() << "xxx";
}
or this
class x {
        logger* m_pLog;
        void AMemberFunc(){
                LGM() << "xxx";
        }
};

-----------------------------------------------------------------
 >> - What is your evaluation of the implementation?

Looks fine. No usage headaches. Find a few minor bugs, to be expected. 
No major ones.

-----------------------------------------------------------------
 >> - What is your evaluation of the documentation?

Good start, but as usual, could use more - John we know you need more 
work :). In particular, I think some description of the underlying 
implementation would be helpful. I'd like to see a description of the 
major objects and their data structures, and their relation to each 
other. I'd like to see a description of how "add_appender" works. It 
took me a while to struggle through this in the code.

In particular, I'd like to see a description of how exactly the 
BOOST_LOG macro works! This is the most important part of the libary, 
and there are couple indirections an inversion in the implementation, 
(which seems fine), but takes a bit to understand.

John, I can volunteer some hours in next couple weeks to help review or 
add doc if you like. If that would be helpful, I just need some 
direction files to review or subject matter. You can e-mail me at "a dot 
schweitzer dot grps at gmail dot com"

-----------------------------------------------------------------
 >> - What is your evaluation of the potential usefulness of the library?

Very useful.

-----------------------------------------------------------------
 >> - Did you try to use the library?  With what compiler?  Did you have any
problems?

I've been using the library at home with VC7.1 We can't currently use it 
at work as we have a compiler that doesn't support nice things like 
boost function. No serious problems. Seems to be working pretty well. 
I've spent 4 - 8 hours a week over last several weeks debugging a 
multi-threaded program with it, and more hours a couple months ago 
testing out its various functions.

-----------------------------------------------------------------
 >> - How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

A quick reading, some usage, then about 8 hours of reading over last 
couple months to understand various aspects.

-----------------------------------------------------------------
 >> - Are you knowledgeable about the problem domain?

More or less. Spent last several years programming an embedded system 
with no debugger or emulator but with a serial port. Printf is my good 
friend. Have spent a some time writing (and putting bugs in) our own 
implementation, which is not nearly so full featured.

Andrew