Re: [lmi] Problem of the week: testing a testing tool

[Greg Chicares]

On 2007-1-10 19:42 UTC, Ericksberg, Richard wrote:
> 
> [timer.cpp]
> 
> + #include "round_to.hpp"
> 
> std::string Timer::elapsed_msec_str() const
> {
>     std::ostringstream oss;
> -   oss << static_cast<int>(1000.0 * elapsed_usec());
> +   round_to<double> const RoundToNearestWhole(0, r_to_nearest);
> +   oss << RoundToNearestWhole(1000.0 * elapsed_usec());
>     oss << " milliseconds";
>     return oss.str().c_str();
> }

We can write it even more simply (I've committed this):

     std::ostringstream oss;
-    oss << static_cast<int>(1000.0 * elapsed_usec());
+    oss << std::fixed << std::setprecision(0) << 1000.0 * elapsed_usec();
     oss << " milliseconds";

Typecasts are for casting types; for truncation, trunc() is better.
(C++98 doesn't have C99's trunc(), but std::floor() would serve as
well here because elapsed times are always positive.) But here we
don't really want truncation at all.

Typecasts are suspicious in general. Casting merely to obtain the
side effect of truncation is just erroneous: it can easily lead to
undefined behavior (C99 6.3.1.4/1; C++98 4.9/1), so a standard-
library function is preferable. Even worse, as in the original
code, is to truncate with a typecast in order to format a floating-
point number with no decimals--where rounding is preferable to
truncation, and the formatting routines perform rounding anyway
(C99 7.19.6.1/8; C++98 22.2.2.2.2/13). Changing a value in order to
produce a side effect in formatting is wrong when the library
provides a natural alternative. (For scaling, the C family has no
builtin support like FORTRAN's 'P' specifier.)

AliquotTimer<F>::operator()() calls the above function:

  << std::scientific << std::setprecision(3)
  << "[" << timer.elapsed_usec() / z << "] " // Scale and round.
  ...
  << timer.elapsed_msec_str()                // Scale and truncate.

and the change I've just committed removes that inconsistency:

  << std::scientific << std::setprecision(3) << "[" << timer.elapsed_usec() / z
  ...
  << std::fixed      << std::setprecision(0) << 1000.0 * elapsed_usec();

How did I ever make such a mistake? All I can guess is that I
learned this anti-idiom a decade or two ago, in a 16-bit world
where linking the floating-point support for printf() made programs
considerably larger.