Logging

[Andrea Crotti]

I need some logging for my application, nothing too complicated, and I
found this (from the link cited at drdobbs)

#ifndef LOG_H
#define LOG_H

#include <stdio.h>
#include <iostream>
#include <sstream>

#define LOG(level) \
if (level > Log::ReportingLevel()) ; \
else Log().Get(level)

// Log, version 0.1: a simple logging class
// taken from http://www.drdobbs.com/cpp/201804215
enum TLogLevel
{
logERROR, logWARNING, logINFO, logDEBUG, logDEBUG1, logDEBUG2, logDEBUG3, logDEBUG4
};

class Log
{
private:
Log(const Log&);
Log& operator =(const Log&);

TLogLevel messageLevel;

public:
Log() {}
virtual ~Log();
std:stringstream& Get(TLogLevel level = logINFO);

public:
static TLogLevel& ReportingLevel();

protected:
std:stringstream os;
};

std:stringstream& Log::Get(TLogLevel level)
{
os << std::string(level > logDEBUG ? 0 : level - logDEBUG, '\t');
messageLevel = level;
return os;
}

Log::~Log()
{
os << std::endl;
fprintf(stderr, "%s", os.str().c_str());
fflush(stderr);
}

#endif /* LOG_H */

which looks perfectly fine for me, BUT this thing is puzzling
static TLogLevel& ReportingLevel();

to actually use this code I should

int main() {
Log::ReportingLevel() = logDEBUG1;
LOG(logWARNING) << "warning";
}

I get the infamous not found symbol
Undefined symbols:
"Log::ReportingLevel()", referenced from:
testLogging() in test.o
testLogging() in test.o
ld: symbol(s) not found
collect2: ld returned 1 exit status

Now a couple of questions
- what can be the problem? (Log.hpp was an header included and that
thing is public)

- is assigning the value of a function in that way good? Doesn't really
look that nice to me...)

- what is the sense of having multiple "public" / "private".
I've seen it in other code also but I don't get why someone should do that...

 

[Öö Tiib]

I need some logging for my application, nothing too complicated, and I
found this (from the link cited at drdobbs)

#ifndef LOG_H
#define LOG_H

#include <stdio.h>
#include <iostream>
#include <sstream>

#define LOG(level) \
    if (level > Log::ReportingLevel()) ;    \
    else Log().Get(level)

// Log, version 0.1: a simple logging class
// taken fromhttp://www.drdobbs.com/cpp/201804215
enum TLogLevel
{
    logERROR, logWARNING, logINFO, logDEBUG, logDEBUG1, logDEBUG2, logDEBUG3, logDEBUG4

};

class Log
{
private:
    Log(const Log&);
    Log& operator =(const Log&);

    TLogLevel messageLevel;

public:
    Log() {}
    virtual ~Log();
    std:stringstream& Get(TLogLevel level = logINFO);

public:
    static TLogLevel& ReportingLevel();

protected:
    std:stringstream os;

};

std:stringstream& Log::Get(TLogLevel level)
{
    os << std::string(level > logDEBUG ? 0 : level - logDEBUG, '\t');
    messageLevel = level;
    return os;

}

Log::~Log()
{
    os << std::endl;
    fprintf(stderr, "%s", os.str().c_str());
    fflush(stderr);

}

#endif /* LOG_H */

which looks perfectly fine for me, BUT this thing is puzzling
    static TLogLevel& ReportingLevel();

to actually use this code I should

int main() {
    Log::ReportingLevel() = logDEBUG1;
    LOG(logWARNING) << "warning";

}

I get the infamous not found symbol
Undefined symbols:
  "Log::ReportingLevel()", referenced from:
      testLogging()     in test.o
      testLogging()     in test.o
ld: symbol(s) not found
collect2: ld returned 1 exit status

Now a couple of questions
- what can be the problem? (Log.hpp was an header included and that
  thing is public)

It looks like Log::ReportingLevel() is declared in header but it is
nowhere defined so linker does not find it.

- is assigning the value of a function in that way good? Doesn't really
  look that nice to me...)

- what is the sense of having multiple "public" / "private".
  I've seen it in other code also but I don't get why someone should do that...

Not sure. It is not forbidden to write "private: private: private:" so
it is up to designer of class.

I usually organize my classes in the way that public section is first,
then comes protected and finally private.

Some have the sections sliced and slices of sections intermixed and so
when later reorganizing them then some slices with same access remain
adjacent. That is perhaps a mess, but mind of most people is a mess
most of the time.

Other reason may be style. For example designer views public type/
constant/enumerator definitions as one section and public member
declarations as other section. The redundant "public:" serves then for
visually separating the two sections.

 

[Andrea Crotti]

It looks like Log::ReportingLevel() is declared in header but it is
nowhere defined so linker does not find it.

Sure but in that example it was not declared somewhere else, and
- if I do everything in a .cpp file, I can't find the definition of the
class
- if I add a definition like
TLogLevel& Log::ReportingLevel() {
return logERROR;
}
in the .cpp file I get

Log.cpp: In static member function ‘static TLogLevel& Log::ReportingLevel()’:
Log.cpp:18: error: invalid initialization of non-const reference of type ‘TLogLevel&’ from a temporary of type ‘TLogLevel’


What I really don't get is why he's using a function (assigning it to
something) instead of just a static variable... What is the gain?

Not sure. It is not forbidden to write "private: private: private:" so
it is up to designer of class.

I usually organize my classes in the way that public section is first,
then comes protected and finally private.

Some have the sections sliced and slices of sections intermixed and so
when later reorganizing them then some slices with same access remain
adjacent. That is perhaps a mess, but mind of most people is a mess
most of the time.

Other reason may be style. For example designer views public type/
constant/enumerator definitions as one section and public member
declarations as other section. The redundant "public:" serves then for
visually separating the two sections.

Well I could understand if you write the multiple "public:" in different
places.
It hides what's really public in the class but well it's a different
layout.

But what sense does it make to do
public:
...
...
public:
....

I normally just use an empty line to divide those parts, it's not
enough?

 

[James Kanze]

Sure but in that example it was not declared somewhere else, and
- if I do everything in a .cpp file, I can't find the definition of the
class
- if I add a definition like
TLogLevel& Log::ReportingLevel() {
return logERROR;
}
in the .cpp file I get

Log.cpp: In static member function ‘static TLogLevel& Log::ReportingLevel()’:
Log.cpp:18: error: invalid initialization of non-const reference of type ‘TLogLevel&’ from a temporary of type ‘TLogLevel’

Return a reference to a static variable.

What I really don't get is why he's using a function
(assigning it to something) instead of just a static
variable... What is the gain?

Managing order of initialization. If the constructor of
a static variable uses your logging facility, it might find that
the variable has not been initialized.

Of course, with an enum type like this, it doesn't matter.
A static variable would be initialized with logERROR. But the
function still allows initialization with any arbitrary default.

Well I could understand if you write the multiple "public:" in
different places. It hides what's really public in the class
but well it's a different layout.

But what sense does it make to do
public:
...
...
public:
....

I normally just use an empty line to divide those parts, it's not
enough?

It doesn't change anything with regards to the compiler. If the
class is logically divided into separate sections, however, it
might make sense to respecify access for each section. I've
often duplicated private: this way, e.g.:

class Toto
{
// ...
private: // virtual functions...
virtual void f() = 0;
// ...

friend class Titi;
private: // Except for Titi...
// ...

private: // really
// ...
};

Obviously, Titi can also access "private: // really". The
comments specify intent, and are not in any way enforced by the
compiler. But somehow, I find this clearer in presentation.

 

[Helge Kruse]

But what sense does it make to do
public:
...
...
public:
....

I normally just use an empty line to divide those parts, it's not
enough?

The compiler doesn't need that empty line. So it's superflous in that mind.

Some guys use empty lines to divide parts, others need lines of '*'. I would
propose to find a coding convention in the group you are programming. So
your eyes gets as much candy as possible.

Helge

 

[Andrea Crotti]

Return a reference to a static variable.

Ok well now I got it, what I still don't get is how to fix this.
I mean if
- I have the definition in the .hpp file, it doesn't find it
- I declare in .hpp file and add a definition in .cpp, it doesn't like
it either

So how am I supposed to write it then?

Managing order of initialization. If the constructor of
a static variable uses your logging facility, it might find that
the variable has not been initialized.

Of course, with an enum type like this, it doesn't matter.
A static variable would be initialized with logERROR. But the
function still allows initialization with any arbitrary default.

I see yes it makes sense then, even if not so intuitive at first sight.

It doesn't change anything with regards to the compiler. If the
class is logically divided into separate sections, however, it
might make sense to respecify access for each section. I've
often duplicated private: this way, e.g.:

class Toto
{
// ...
private: // virtual functions...
virtual void f() = 0;
// ...

friend class Titi;
private: // Except for Titi...
// ...

private: // really
// ...
};

Obviously, Titi can also access "private: // really". The
comments specify intent, and are not in any way enforced by the
compiler. But somehow, I find this clearer in presentation.

Yes sure it's a matter of style also and maybe it dependes on the
particular project also...

And another thing, why the documentation (doxygen style) is normally in
the implementation?

I mean, if I read someone else code then normally I read first the
header, and thus I don't gain much about how it really works.
Unless of course I find where the function is defined or I use doxygen,
which maybe sometimes is not possible...

 

[Vladimir Jovic]

Ok well now I got it, what I still don't get is how to fix this.
I mean if
- I have the definition in the .hpp file, it doesn't find it
- I declare in .hpp file and add a definition in .cpp, it doesn't like
it either

So how am I supposed to write it then?

When a method (or function) returns a reference, then it can not returns
a reference to a temporary, like this :

struct A{
};

A& foo()
{
A a;
//...
return a;
}

However, you can do this :
A& foo()
{
static A a;
//...
return a;
}

or this :

struct B
{
A& foo()
{
// ...
return a;
}

A a;
};

 

[Öö Tiib]

And another thing, why the documentation (doxygen style) is normally in
the implementation?

It often is, but it is not mandatory. Up to team to agree how and
where and if.

I mean, if I read someone else code then normally I read first the
header, and thus I don't gain much about how it really works.
Unless of course I find where the function is defined or I use doxygen,
which maybe sometimes is not possible...

User of class who has read the documentation might need to look into
header when forgot some name or other little detail. Doxygen comments
can be then needless distraction and clutter.

Writer, maintainer or reviewer of class implementation benefit from
such comments close to definition since it helps to verify that the
function is doing what documentation claims. Close things are easier
to fix when these do not match. Adding some detail or typo fix to
comments does not cause everything to rebuild that uses the class.

The comments within header might be beneficial when there are/will be
no documentation generated from code or the reader of code does not
want to read such documentation ever. On such case the doxygen
comments are somewhat pointless, normal comments are much better to
read for non-robots.

 

[James Kanze]

Ok well now I got it, what I still don't get is how to fix this.
I mean if
- I have the definition in the .hpp file, it doesn't find it
- I declare in .hpp file and add a definition in .cpp, it doesn't like
it either

So how am I supposed to write it then?

What sort of error are you getting? For what code?

If it is a template, declare and define everything in the
header. Otherwise, define the class and declare any free
functions in the header, and define the functions in the source
file. If this doesn't work, you're doing something else wrong.

[...]

And another thing, why the documentation (doxygen style) is
normally in the implementation?

It isn't. The general rule is to put external documentation in
the header, and internal in the source files. If both are
doxygen, you can use different Doxyfiles to generate the two
types of documentation: the one for external documentation just
looks at the headers; the one for internal looks at both.

 

[Jorgen Grahn]

.

And another thing, why the documentation (doxygen style) is normally in
the implementation?

I mean, if I read someone else code then normally I read first the
header, and thus I don't gain much about how it really works.

If you read my header files, you'll get the main documentation for each
class, plus the names and signatures of their public members.
Hopefully that's a decent balance between brevity and information.

Unless of course I find where the function is defined or I use doxygen,
which maybe sometimes is not possible...

If it's not possible, then you're not "reading someone else's code" --
you are a paying customer of some closed-source library, and should
demand some pretty good documentation in the form of man pages, PDFs,
Doxygen-generated HTML ... Most code doesn't fall in that category.
(Personally I can't remember when I last used such a library -- in the
early 1990s, perhaps.)

I also agree with everything Öö Tiib wrote elsewhere.

/Jörgen