views:

183

answers:

6

hi guys,

I'm writting a log class in c++. This class is an singleton. I want to add logs in such a way:

Log::GetInstance() << "Error: " << err_code << ", in class foo";

Ok, and inside a Log object, I want to save this whole line at the time when the last argument comes (", in class foo" in this example).

How to detect the last one << argument? << a << b << is_this_last << maybe_this_is << or_not.

I dont to use any end tags.

+14  A: 

You can solve this problem by not using a singleton. If you make a function like this:

Log log()
{
    return Log();
}

You can add a log almost the same way you did before:

log() << "Error: " << err_code << ", in class foo";

The difference is that the destructor of the Log object gets called after this line. So now you have a way to detect when the last argument has been processed.

Job
Some object somewhere still has to maintain the open file handle…
Potatoswatter
@Potatoswatter: Yes. Since he was using a singleton, I'd say making it a static member of the `Log` class. Other solutions are of course possible.
Job
I do like RAII techniques playing with constructor and destructor, I think it's really elegant.
Stephane Rolland
+9  A: 

I would have your Log::GetInstance return a proxy object instead of the log object itself. The proxy object will save the data that's written to it, and then in its destructor, it'll actually write the accumulated data to the log.

Jerry Coffin
+1: Although singletons may be ugly, it's better to give advice that requires less unnecessary refactoring.
Potatoswatter
+1  A: 

Don't get too clever with your operators. You should overload operators when it makes sense to do so. Here you don't should not. That just looks weird.

You should just have a static method that looks like this:

Log::Message( message_here );

which takes a std::string. Then clients have the head-ache of figuring out how to assemble the error string.

C Johnson
Why is it weird? C++ streams work in exactly the same way.
Job
@Job: No, C++ streams don't. Specifically, they do nothing special at the last invocation. A C++ stream object uses `operator<<()` to run a function of one argument, and returns a C++ stream object that can again run a function. Operator overloading is neat, but it isn't very flexible or extendable, and there are cases where adding another requirement makes operator overloading a Bad Idea.
David Thornley
@David: But the answer isn't talking about what `operator<<` does, that just doing `operator<<` in the first place is weird. Which it isn't.
GMan
@GMan: I disagree. Log files have been used for a long time, with `operator<<()`, and working just like standard C++ streams. Few people consider that weird. The issue is the extra requirement at the end of the statement.
David Thornley
@David: But few people care how the insertion *works*. The need to do something at the end of the insertion chain is an implementation detail, and has nothing to do with `operator<<`. If C++ streams returned a proxy and then did some necessary thing at the end of the chain, would you notice? Would you care?
GMan
@GMan: The OP wanted to do something at the end of the line, apparently without further action on his part. You may call this an implementation detail, but it is just such implementation details that make operator overloading a bad idea in some situations. The fact that the OP is having trouble doing what he wants to do with operator overloading suggests that overloading is a bad idea. As for whether I'd notice if C++ streams worked that way...yes, I'd notice big-time and probably with vulgar words whenever I had to write my own `operator<<()`.
David Thornley
@David: His having trouble with something just means his knowledge could be expanded, and has no bearing on a language feature. Once almost *everyone* gets confused at something, then we'll call using the language feature a bad idea. If he returned a reference to a local variable and asked why his program crashed, I'm sure you wouldn't say "guess you shouldn't use references." How would it make writing your own `operator<<` any harder? Just change `std::ostream` into `std::ostream_proxy` and you're done. (Obviously the latter isn't standard.) And my point was with usage, you wouldn't be...
GMan
GMan
@GMan: With many language features, not understanding how it applies means your understanding is flawed. With operator overloading, not understanding how to do something means DON'T USE OPERATOR OVERLOADING. Operator overloading should be transparent in use, or it's a really bad idea. Doing anything clever with it can make a program unreadable real fast, so if you think you have to do something clever you shouldn't overload operators. How an overloaded operator does its thing is not transparent to anybody who has to understand the code, or write a new operator.
David Thornley
@David: An answer that says "don't do it" instead of "here's how" is bad in my books. Not sharing knowledge is a terrible thing to do, to me. When someone wants to do something *a bad way* ("what's the cleanest way to typedef a pointer?" -> "don't [because it's always confusing]"), that's fine, but you or I could easily make this interface simple to use, so why not spread that around? I don't know, your only argument seems to be "someone might not get it, so don't do it." So nobody should write any code, because not everyone understands code? Why special plead for operators?
GMan
@GMan: Typedefing a pointer is frequently a good thing. Overloading operators to do things that aren't intuitive is almost always a Bad Thing. In this, I'm following "C++ Coding Standards" by Sutter and Alexandrescu, Chapter 26 "Preserve Natural Semantics for Overloaded Operators", and FAQ 2.14 of Cline, Lomow, and Gou's "C++ FAQs". I don't like to refer to books here, but this comment thread is no place to argue what constitutes abuse of overloaded operators.
David Thornley
@David: Ew, do you mean `typedef some_type* some_type_ptr`? The only time that's generally good is with a `shared_ptr` or something, but to mark a single asterisk is confusing. I feel you're not separating implementation from interface, can we agree that distinction exists? Operators provide an *interface*. It's no more unintuitive to do what the OP wants (insert some data into a stream with `operator<<`) than it is to use C++ streams (insert some data into a stream with `operator<<`.) What's the difference in interface? Saying "OP's detects the end and does something" is *not* a change...
GMan
...in interface, it's used exactly the same. His use of `operator<<` in the interface is a perfectly fine choice, like @Job says. The only thing left is the implementation. We both know *lots* of stuff goes on under the hood in C++ streams to get `operator<<` to work, and we're fine with that, the *interface* is the same. Likewise, with OP's code returning a proxy to do that under the hood work, the *interface* is the same. Nothing in an interface is effected by an implementation detail. Nothing about his operator use or interface leads us to conclude his interface is bad, it's exactly the...
GMan
...same as C++ streams. The implementation is obviously different, as it proxies insertions, but that's just a layer of abstraction. You can't argue that's bad, and even if you did you would no longer be arguing about the operator use, which is in the *interface*. You could say "make users terminate the line with `endl`, not detect it.", but note nothing in that sentence has anything to do with the operator. And I disagree with that anyway, making an interface easy to use is a top priority, and in the same way we don't want people to have to remember to `delete`, it would be nice for his...
GMan
...users to not have to remember `endl`. And there's an extremely simple solution for that, so why not use it? The question is about implementation, the `operator<<` belongs to the interface, so mentioning it isn't answering the question.
GMan
@GMan: If you don't see the difference, it's too much to deal with in a comment thread. Sorry, if you want to discuss this further with me it will have to be with better facilities.
David Thornley
I dont think Log::Message( std::string message_here ); is a good idea... you can't add numbers to std::string, so operations like:Log::Message("Error code: " + code); are impossible (of course without overloading this operation).
asm
+4  A: 

You make Log return a different object after the operator << .

template<typename T>
LogFindT operator<<(Log aLog, T const& data)
{
    // Put stuff in log.
    log.putStuffInLog(data);

    // now return the object to detect the end of the statement.
    return LogFindT(aLog);
}


struct LogFindT
{
    LogFindT(Log& aLog) : TheLog(aLog) {}
    Log& TheLog;
    ~LogFindT()
    {
        // Do stuff when this object is eventually destroyed
        // at the end of the expression.
    }
};

template<typename T>
LogFindT& operator<<(LogFindT& aLog, T const& data)
{
     aLog.TheLog.putStuffInLog(data);

     // Return a reference to the input so we can chain.
     // The object is thus not destroyed until the end of the stream.
     return aLog;
}
Martin York
+4  A: 

I think Jerry and Martin have given the best suggestion, but for the sake of completeness, the first thing I thought of was std::endl.

If you implemented Log within the iostream system by a custom streambuf class, then you can simply add << endl or << flush at the end of the line. Since you're asking, I suppose you didn't.

But you can mimic the way endl works. Either add a manipulator handler

Log &operator<< ( Log &l, Log & (*manip)( Log & ) )
    { return manip( l ); } // generically call any manipulator

Log &flog( Log &l ) // define a manipulator "flush log"
    { l->flush(); return l; }

or add a dedicated operator<<

struct Flog {} flog;

Log &operator<< ( Log &l, Flog )
    { l->flush(); return l; }
Potatoswatter
A: 

There's no good way to do what you want. C and C++ are simply not line-oriented languages. There is no larger unit like a "line of code" or anything, nor are chained calls combined in any way.

In C++ the expression "a << b << c << d" is exactly equivalent to three separate calls to operator<<, like this:

 t1 = a;
 t2 = t1.operator<<(b);
 t3 = t2.operator<<(c);
 t4 = t3.operator<<(d);

That's why C++ ostreams use endl as an explicit end-of-line marker; there's just no decent way to do it otherwise.

Drew Thaler
BTW, I appreciate all the other answers which came up with ways to do it. Creating short-lived temporaries and proxies and such is a fun academic diversion, but in practice it's going to create extra runtime overhead and complicate the debugging (of your logging and debugging system!) if anything goes wrong.I didn't really want to go there, because if you don't understand it well enough to come up with the idea yourself, it's just a more complicated way to shoot yourself in the foot.So I still stand by my assertion - there's no *good* way to do what you want. :-)
Drew Thaler
So basically instead of teaching them you say "you couldn't figure it out, don't try."? Also, these proxies will be fully inlined by any compiler made in the last decade, there's no overhead. Even if there was, a clean interface is better than a fast one. (One can always make a clean interface fast *with a profiler* (not guessing), but a fast interface is much harder to make clean.)
GMan
I disagree (with Drew). Creating temporaries is not an academic practice but seen quite often in commercial code. There is no significant overhead (another one of those internet rumors) and it is not that hard to debug (as it is relatively easy to get write first time (so no errors)).
Martin York