Code review - Issue 583320043: Issue 5659: Clean up to_string () etc.https://codereview.appspot.com/2020-01-12T19:30:07+00:00rietveld
Message from unknown
2020-01-11T20:04:56+00:00Dan Ebleurn:md5:adece86b79922abe3767c7e836f9270e
Message from unknown
2020-01-11T20:05:28+00:00Dan Ebleurn:md5:bfdbdcba1c43832dd88535d54645326d
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:05:30+00:00Dan Ebleurn:md5:0840915c029ab54f64ec2d8ff29e20e5
Remove String_convert::char_string
Message from unknown
2020-01-11T20:06:21+00:00Dan Ebleurn:md5:ed6900e75dc9855ddabe3a16d78876d1
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:06:22+00:00Dan Ebleurn:md5:1e13cc6c7907910a38dc3949445f706e
Delete ::to_string (const string&)
Message from unknown
2020-01-11T20:06:49+00:00Dan Ebleurn:md5:15f36e2990723679b242017ebb810685
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:06:55+00:00Dan Ebleurn:md5:6a1854bc58e8884939c3fdf9266184a5
Delete ::to_string(char, ssize_t)
Message from unknown
2020-01-11T20:07:16+00:00Dan Ebleurn:md5:8868ed9dc0f8fffb56747d5064252676
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:07:17+00:00Dan Ebleurn:md5:f1373e01f69b816dba8790f1673bec8c
Delete ::to_string (bool b)
Message from unknown
2020-01-11T20:07:47+00:00Dan Ebleurn:md5:2c8efcd08474440c87c212a0389060b1
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:07:58+00:00Dan Ebleurn:md5:2e584878e1f14de7b181240e6e49d83f
Delete ::to_string (i) for various integer types
Message from unknown
2020-01-11T20:08:35+00:00Dan Ebleurn:md5:4c22478c92cdb09e697ecc3b770d2a98
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:08:38+00:00Dan Ebleurn:md5:688e956507e7bb3187a17bd876094049
Finally remove deleted ::to_string () overloads
Message from unknown
2020-01-11T20:09:22+00:00Dan Ebleurn:md5:69730a3dd8e2dfc32eeab32f5ee9ffb9
Message from nine.fierce.ballads@gmail.com
2020-01-11T20:09:26+00:00Dan Ebleurn:md5:ac15227807afebfe5c6cd14e85589fbd
Remove Interval<T>::T_to_string ()
Message from lemzwerg@googlemail.com
2020-01-11T22:20:35+00:00lemzwergurn:md5:29e55fe1ee086281181b369e946b5a4e
LGTM, thanks!
https://codereview.appspot.com/583320043/diff/581420044/flower/file-name.cc
File flower/file-name.cc (right):
https://codereview.appspot.com/583320043/diff/581420044/flower/file-name.cc#newcode124
flower/file-name.cc:124: s += ext_;
any reason to use two lines? In other files you use a single line for similar things.
Message from nine.fierce.ballads@gmail.com
2020-01-12T00:20:03+00:00Dan Ebleurn:md5:7d09471497aa5c0a43d61f1085ae2583
https://codereview.appspot.com/583320043/diff/581420044/flower/file-name.cc
File flower/file-name.cc (right):
https://codereview.appspot.com/583320043/diff/581420044/flower/file-name.cc#newcode124
flower/file-name.cc:124: s += ext_;
On 2020/01/11 22:20:34, lemzwerg wrote:
> any reason to use two lines? In other files you use a single line for similar
> things.
s += EXTSEP; // append a char to s
s += ext_; // append a string to s
— vs. —
s += EXTSEP + ext_;
\___________/
make a string
\________________/
append it to s
Either one could involve memory allocation, but I deemed it more
likely in the latter case. I don't have reason to suspect this is a
performance bottlenck, but given that I had to touch the line, and
that it wasn't a major rewrite, I decided to try to improve it.
Message from lemzwerg@googlemail.com
2020-01-12T07:44:42+00:00lemzwergurn:md5:0886132467dc5c096a3d41e9b6568232
> s += EXTSEP; // append a char to s
> s += ext_; // append a string to s
>
> — vs. —
>
> s += EXTSEP + ext_;
> \___________/
> make a string
> \________________/
> append it to s
>
> Either one could involve memory allocation, but I deemed it more
> likely in the latter case. I don't have reason to suspect this is a
> performance bottlenck, but given that I had to touch the line, and
> that it wasn't a major rewrite, I decided to try to improve it.
I'm not sure that it is an improvement. Today, compilers use very good optimization tools; I can imagine that the performance of both lines is exactly the same – or that the one-line version performs even better since the compiler can handle the memory allocation internally in an optimized way. Only a profiler can give a clear answer, and this is most certainly dependent on the platform and the compiler.
I thus favor code that is easiest to understand by the user, or which looks nice. Normally, I very much favor a vertical flow of source code, but in this particular case the compact one-liner looks easier to comprehend.
Message from unknown
2020-01-12T19:30:05+00:00Dan Ebleurn:md5:c1560ab37061aa563efe711da0fb5477
Message from nine.fierce.ballads@gmail.com
2020-01-12T19:30:07+00:00Dan Ebleurn:md5:1039daf99e5695965d955845e1814dc1
Werner's feedback