Re: [lmi] [lmi-commits] master 86aacf1 5/5: Clear error flags when resetting stringstream contents

[Vadim Zeitlin]

On Fri, 3 Mar 2017 15:20:41 +0000 Greg Chicares <address@hidden> wrote:

GC> [icedove froze, so...rewriting my response; sorry if you get two
GC> different replies, but if you do, they should say materially the
GC> same thing]

[no, I didn't and received just this one]

GC> > GC> diff --git a/rtti_lmi_test.cpp b/rtti_lmi_test.cpp
GC> > GC> index 65eb3c6..4d1d75e 100644
GC> > GC> @@ -66,6 +66,7 @@ void RttiLmiTest::TestTypeInfo()
GC> > GC>      ti1 = typeid(X);
GC> > GC> +    oss.clear();
GC> > GC>      oss.str("");
GC> > GC>      oss << ti1;
GC> > GC>      BOOST_TEST_EQUAL(oss.str(), 
lmi::detail::Demangle(typeid(X).name()));
GC> > 
GC> >  And here it looks like we only use "oss" to output a string to it, so
GC> > normally it shouldn't have any state bits set...
GC> > 
GC> >  What am I missing?
GC> 
GC> Error-proofing.

 OK, but may I suggest adding a comment saying that this statement is not
needed but is there just in case it becomes needed one day? Because I think
I might be not the only one to see this apparently unnecessary line and
waste time trying to understand why is it actually necessary.

 In fact, maybe the best would be to just stop using the same ostringstream
object at all? This would avoid avoid the need for both the new line and
.str("") call following it and just replace it with a line constructing
another ostringstream object. This is probably a tiny bit less efficient,
but we definitely don't care about this in the unit test and probably don't
care about it when updating the progress meter neither. Of course, in that
other case, knowing that we're in wx-using code anyhow, I'd replace the
lines

        oss << "Waiting " << 0.1 * i << " seconds";
        progress_dialog_.Update(count(), oss.str());

with an even simpler, IMO

        progress_dialog_.Update
                (count()
                ,wxString::Format("Waiting %.1f seconds", 0.1 * i)
                );

VZ