Move get_normal to Offset::normal

Move get_normal to Offset::normal

[dak, 2020-04-13 18:24:47]

https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh
File flower/include/offset.hh (right):

https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset...
flower/include/offset.hh:123: Offset normal() const {
It's kind of unusual to make a constructing function a (non-static) member
function unless this is necessitated for operator semantics, because different
conversion rules apply and there is some expectation that member functions
delivering the class type may change stuff in-place (the const is not seen at
call sites).

Any reason to depart from conventions here?

[hanwenn, 2020-04-13 19:08:31]

https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh
File flower/include/offset.hh (right):

https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset...
flower/include/offset.hh:123: Offset normal() const {
On 2020/04/13 18:24:47, dak wrote:
> It's kind of unusual to make a constructing function a (non-static) member
> function unless this is necessitated for operator semantics, because different
> conversion rules apply and there is some expectation that member functions
> delivering the class type may change stuff in-place (the const is not seen at
> call sites).
> 
> Any reason to depart from conventions here?

It's consistent with offset::direction, offset::swapped, but I followed your
suggestion.

[hanwenn, 2020-04-13 19:08:57]

dak

[dak, 2020-04-13 19:38:02]

On 2020/04/13 19:08:31, hanwenn wrote:
>
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh
> File flower/include/offset.hh (right):
> 
>
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset...
> flower/include/offset.hh:123: Offset normal() const {
> On 2020/04/13 18:24:47, dak wrote:
> > It's kind of unusual to make a constructing function a (non-static) member
> > function unless this is necessitated for operator semantics, because
different
> > conversion rules apply and there is some expectation that member functions
> > delivering the class type may change stuff in-place (the const is not seen
at
> > call sites).
> > 
> > Any reason to depart from conventions here?
> 
> It's consistent with offset::direction, offset::swapped, but I followed your
> suggestion.

You are right regarding direction and swapped, so your proposed change was not
creating precedent and with the reason of maintaining convention/consistency
untenable, I think I prefer your originally proposed syntax.  Sorry for the
noise.

Any input from our C++ gurus?

[hanwenn, 2020-04-13 20:08:51]

On 2020/04/13 19:38:02, dak wrote:
> On 2020/04/13 19:08:31, hanwenn wrote:
> >
>
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset.hh
> > File flower/include/offset.hh (right):
> > 
> >
>
https://codereview.appspot.com/576000043/diff/577750046/flower/include/offset...
> > flower/include/offset.hh:123: Offset normal() const {
> > On 2020/04/13 18:24:47, dak wrote:
> > > It's kind of unusual to make a constructing function a (non-static) member
> > > function unless this is necessitated for operator semantics, because
> different
> > > conversion rules apply and there is some expectation that member functions
> > > delivering the class type may change stuff in-place (the const is not seen
> at
> > > call sites).
> > > 
> > > Any reason to depart from conventions here?
> > 
> > It's consistent with offset::direction, offset::swapped, but I followed your
> > suggestion.
> 
> You are right regarding direction and swapped, so your proposed change was not
> creating precedent and with the reason of maintaining convention/consistency
> untenable, I think I prefer your originally proposed syntax.  Sorry for the
> noise.
> 
> Any input from our C++ gurus?

offset_directed follows your suggestion, though.

[hanwenn, 2020-04-15 18:23:26]

revert to method