ticket:12140: Pass StringPiece by value, return value.

ticket:12140: Pass StringPiece by value, return value.

R=markus.icu@gmail.com

Committed: http://bugs.icu-project.org/trac/changeset/39276

[markus.icu, 2016-08-04 17:53:14]

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
File source/common/unicode/bytestrie.h (right):

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
source/common/unicode/bytestrie.h:311: StringPiece getString() const { return
sp_; }
Could the Iterator implementation get any simpler/shorter by removing the sp_
field and constructing the return value on the fly?

[markus.icu, 2016-09-12 17:39:26]

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
File source/common/unicode/bytestrie.h (right):

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
source/common/unicode/bytestrie.h:311: StringPiece getString() const { return
sp_; }
On 2016/08/04 17:53:14, markus.icu wrote:
> Could the Iterator implementation get any simpler/shorter by removing the sp_
> field and constructing the return value on the fly?

Ping. Did this get resolved?

[roubert (google), 2016-09-16 15:34:45]

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
File source/common/unicode/bytestrie.h (right):

https://codereview.appspot.com/305010043/diff/1/source/common/unicode/bytestr...
source/common/unicode/bytestrie.h:311: StringPiece getString() const { return
sp_; }
On 2016/08/04 17:53:14, markus.icu wrote:
> Could the Iterator implementation get any simpler/shorter by removing the sp_
> field and constructing the return value on the fly?

Yes, that's possible and will make the code both simpler and faster. See now
updated patch set.

[markus.icu, 2016-09-16 16:10:29]

LGTM

[roubert (google), 2016-09-16 18:23:09]

On 2016/09/16 16:10:29, markus.icu wrote:
> LGTM

So, what would the proper procedure for merging this change be? Should I just
commit is as a follow-up to the exitsing approved design proposal for ticket
#12140 or should some other path be followed?

[markus.icu, 2016-09-16 18:53:04]

On Fri, Sep 16, 2016 at 11:23 AM, <roubert@google.com> wrote:

> On 2016/09/16 16:10:29, markus.icu wrote:
>
>> LGTM
>>
>
> So, what would the proper procedure for merging this change be? Should I
> just commit is as a follow-up to the exitsing approved design proposal
> for ticket #12140 or should some other path be followed?
>

No design update. The design does not change with this internal
implementation refinement.
Please submit to the trunk and mark the ticket as fixed (since I have
reviewed it already).

markus

[roubert (google), 2016-09-19 11:08:02]

Committed patchset #2 (id:20001) manually as 39276 (presubmit successful).