Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Please take a look at this code:

class error_stream: boost::noncopyable
{
public:
    template<typename T>
    std::ostream& operator<<(T&& arg)
    {
        return strm() << std::forward<T>(arg);
    }
    std::string str() { return strm().str(); }
    bool empty() const { return !holder_.valid(); }
private:
    std::ostringstream& strm() { return holder_; }
    struct alignas(std::ostringstream) holder
    {
        holder(): ptr_{} {}
        ~holder()
        {
            if(valid())
                ptr_->~basic_ostringstream();
        }
        operator std::ostringstream&()
        {
            if(!valid())
                ptr_ = new (memory_) std::ostringstream{};
            return *ptr_;
        }
        bool valid() const { return ptr_; }
    private:
        char memory_[sizeof(std::ostringstream)];
        std::ostringstream* ptr_;
    } holder_;
};

Here I want to avoid creating std::ostringstream if there was no output made into it:

error_stream strm;
//strm << "...";
if(!strm.empty())
{
    //...get strm.str();
}

I use placement new/delete when a client outputs something into the stream. Do I need to use alingas for block of memory that will hold instance of std::ostringstream?

UPDATE: To give more context - this is part of THROW wrapper:

...
#define THROW_EX_WITH_LOCATION(EXCEPTION, LOCATION)                \
    for(error_stream strm;;                                        \
        strm.empty()?                                              \
            throw_exception<EXCEPTION>(LOCATION):                  \
            throw_exception<EXCEPTION>(LOCATION, strm.str()))      \
        strm
...

I don't want to create any stream if ther is no error message but solution should work with minumum overhead over version with just std::ostringstream.

Consider the following cases:

THROW();

and

THROW() << "some error";

I agree this looks like nitpicking.

share|improve this question

1 Answer 1

up vote 6 down vote accepted

This seems overcomplicated to me. Lazy-loading something doesn't require placement-new trickery and mucking around with alignments. Why not simply store a pointer to the stream which is only initialised when the user performs some kind of operation? This is the general way to do lazy-loading:

struct error_stream
{
private:
    std::unique_ptr<std::ostringstream> stream;

    // This should use make_unique, which you can find 
    // or write yourself without too much hassle.
    void construct()
    {
        stream = std::unique_ptr<std::ostringstream>(
            new std::ostringstream);
    }

public:

    template <typename T>
    std::ostream& operator<<(T&& arg)
    {
        if(!stream) {
            construct();
        }
        (*stream) << std::forward<T>(arg);
        return *stream;
    }

    bool empty() const
    {
        return stream ? false : true;
    }

    std::string str() 
    {
        if(empty()) {
             construct();
        }
        return stream->str();
    }
};

This seems to have the semantics you want (including being non-copyable due to using a std::unique_ptr) with much less hassle. You don't have to worry about placement new or any alignment requirements, which is going to make the code a -lot- harder to maintain.

Edit: If you really what to avoid the heap, I suppose you can use placement new. You allocate the memory for the stream when you create an error_stream, so I'm curious as to what problem you're trying to solve exactly. That being said, what you want to align is the storage for the ostringstream.

char memory_[sizeof(std::stringstream)];

Hence I'd use:

struct holder
{
....
private:
    alignas(std::ostringstream) char memory_[sizeof(std::stringstream)];
    ...
};
share|improve this answer
    
Yes your solution is cleaner. I'm trying to avoid going to free store if possible. –  AlexT Feb 20 at 5:21
    
@AlexT See my edit. –  Yuushi Feb 20 at 5:36

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.