lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Greg Chicares
Subject: Re: [lmi] [lmi-commits] master 86aacf1 5/5: Clear error flags when resetting stringstream contents
Date: Sun, 12 Mar 2017 23:37:13 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0

On 2017-03-03 18:56, Vadim Zeitlin wrote:
> On Fri, 3 Mar 2017 15:20:41 +0000 Greg Chicares <address@hidden> wrote:
[...]
> 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.

Oh. I thought that was a standard idiom for resetting an ostringstream.
But the code can be improved by not (re)using 'oss' here:
  commit 9d5c76eadb98f79a48f19dcc4910291091e3d96a

>  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)
>               );

Here our subjective realities differ in that you remember how to read
printf format codes, but I do not. I think maybe '.1' tells printf
to show one decimal place, but I'd have to look that up to be sure.
And is 'i' floating point anyway? Oh, now I see: '0.1 * i' must be
floating point. And if 'i' equals ten, then does '%.1f' print "10",
or "10.0", or maybe "10."? I can't remember. OTOH, the C++ stream
code feels maximally simple to me.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]