> On Jan. 5, 2015, 10:47 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp, lines 70-71
> > <https://reviews.apache.org/r/29533/diff/1/?file=805385#file805385line70>
> >
> > Shouldn't this be an output stream operator instead of stringify? (e.g.
> > Duration / Bytes). That way, you can do both of the following:
> >
> > ```
> > stringify(url);
> > LOG(INFO) << url;
> > ```
> >
> > As opposed to requiring the explicit stringify call:
> > ```
> > LOG(INFO) << stringify(url);
> > ```
>
> Benjamin Hindman wrote:
> Okay, but what's so bad about being explicit!? ;-) The original intention
> of stringify was to have overloads (hence all the overloads in stringify.hpp)
> and then be explicit in all of the stringifications. In otherwords, when
> would we ever use stringify with LOG?
>
> Ben Mahler wrote:
> Thanks!
>
> > The original intention of stringify was to have overloads (hence all
> the overloads in stringify.hpp) and then be explicit in all of the
> stringifications.
>
> I see, I do enjoy having 1 way to do it, but are you thinking of actually
> sweeping the code to remove all of our `<<` operators?
> It also sounds painful when it comes to dealing with external
> abstractions that print using `<<`?
Personally I think it would be better to have `operator<<`s for all types that
we want to be able to print and have a single templated `stringify`.
The difference becomes more apparent with `strm << x << " " << z` vs. `strm <<
stringify(x) + " " + stringify(y)`.
The first example simply writes directly to the stream, no unnecessary strings
being constructed. In the second, there are 5 unnecessary strings being
constructed.
1. `stringify(x)`, 2. `""` becomes a `std::string` to do concat, 3.
`stringify(y)`, 4. `(1) + (2)`, 5. `((1) + (2)) + (3)`.
I don't think we should have to construct temporary strings in order to write
to a value to a stream. I think `stringify` should only be used when we
actually need to give rise to a `string` which isn't often.
The most used case of `stringify` is actually in constructing `Error` objects
to form error messages, which are constructed like the second example from
above, something like: `Error("Some error: " + stringify(x) + " isn't " +
stringify(y) + '.')`. The streaming version would look like:
`Error((std::ostringstream() << "Some error: " << x << " isn't " << y <<
'.').str())`. The good news is that it doesn't have to be this ugly!
We could introduce a `concat` in which case it can look like:
`Error(concat("Some error: ", x, " isn't ", y, '.'))`.
The implementation of `concat` is trivial: `template <typename... Ts>
std::string concat(const Ts&...ts) { return join("", ts...); }` and `join` uses
the streaming pattern already. Depending on the syntactic preference we could
even do: `Error(concat() << "Some error: " << x << " isn't " << y << '.')`
This can also have a trivial implementation: `struct concat :
std::ostringstream { operator std::string() const { return str(); } };`
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29533/#review66739
-----------------------------------------------------------
On Jan. 21, 2015, 5:44 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29533/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2015, 5:44 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am
> 8e51957d141af0be64cac42f65e03bca5929c8a9
> 3rdparty/libprocess/3rdparty/stout/include/stout/url.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/url_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/29533/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
