Re: Move get_normal to Offset::normal (issue 576000043 by address@hidden)

[hanwenn]

Reviewers: dak,


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.hh#newcode123
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.

Description:
Move get_normal to Offset::normal

Please review this at https://codereview.appspot.com/576000043/

Affected files (+19, -24 lines):
  M flower/include/offset.hh
  M lily/stencil-integral.cc


Index: flower/include/offset.hh
diff --git a/flower/include/offset.hh b/flower/include/offset.hh
index 
955b4a226bff213178365381cc3ad16b43858f41..a2d62cf01d6c69126cb82fa9f8dae8925de50f3f
 100644
--- a/flower/include/offset.hh
+++ b/flower/include/offset.hh
@@ -115,6 +115,14 @@ public:
   Real length () const;
   bool is_sane () const;
   Offset operator *= (Offset z2);
+
+  /*
+    Gets an orthogonal vector with same size to orig, pointing left
+    (in the complex domain, a multiplication by i)
+  */
+  Offset normal() const {
+    return Offset(-coordinate_a_[Y_AXIS], coordinate_a_[X_AXIS]);
+  }
 };
 
 #include "arithmetic-operator.hh"
@@ -175,4 +183,3 @@ cross_product (Offset o1, Offset o2)
 }
 
 #endif /* OFFSET_HH */
-
Index: lily/stencil-integral.cc
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index 
6f4dfbae3fee4e764a9a308f290d009e2bb5fae9..6e667992a1af109a9e531c29e91754fd834f5e4a
 100644
--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -128,15 +128,6 @@ get_path_list (SCM l)
   return SCM_BOOL_F;
 }
 
-// Gets an orthogonal vector with same size to orig, pointing left
-// (in the complex domain, a multiplication by i)
-
-Offset
-get_normal (Offset orig)
-{
-  return Offset (-orig[Y_AXIS], orig[X_AXIS]);
-}
-
 //// END UTILITY FUNCTIONS
 
 /*
@@ -172,7 +163,7 @@ make_draw_line_boxes (vector<Box> &boxes, 
vector<Drul_array<Offset> > &buildings
   Offset dir = (right - left).direction ();
   for (DOWN_and_UP (d))
     {
-      Offset outward = d * get_normal ((thick / 2) * dir);
+      Offset outward = (d * thick / 2) * dir.normal ();
       Offset inter_l = scm_transform (trans, left + outward);
       Offset inter_r = scm_transform (trans, right + outward);
       if ((inter_l[X_AXIS] == inter_r[X_AXIS]) || (inter_l[Y_AXIS] == 
inter_r[Y_AXIS]))
@@ -268,7 +259,7 @@ make_partial_ellipse_boxes (vector<Box> &boxes,
           Real ang
             = linear_map (start, end, 0, quantization, static_cast<Real> (i));
           Offset pt (offset_directed (ang).scale (rad));
-          Offset inter = t (pt + d * get_normal ((th / 2) * pt.direction ()));
+          Offset inter = t (pt + (d * th / 2) * pt.direction ().normal ());
           points[d].push_back (inter);
         }
     }
@@ -299,21 +290,11 @@ make_partial_ellipse_boxes (vector<Box> &boxes,
     {
       // beg line cap
       Offset pt (offset_directed (start).scale (rad));
-      create_path_cap (boxes,
-                       buildings,
-                       trans,
-                       pt,
-                       th / 2,
-                       -get_normal (pt));
+      create_path_cap (boxes, buildings, trans, pt, th / 2, -pt.normal ());
 
       // end line cap
       pt = offset_directed (end).scale (rad);
-      create_path_cap (boxes,
-                       buildings,
-                       trans,
-                       pt,
-                       th / 2,
-                       get_normal (pt));
+      create_path_cap (boxes, buildings, trans, pt, th / 2, pt.normal ());
     }
 }
 
@@ -440,6 +421,13 @@ create_path_cap (vector<Box> &boxes,
                                           SCM_UNDEFINED));
 }
 
+// TODO - remove once https://codereview.appspot.com/583750044/ is in.
+Offset
+get_normal (Offset o)
+{
+  return o.normal ();
+}
+
 void
 make_draw_bezier_boxes (vector<Box> &boxes,
                         vector<Drul_array<Offset> > &buildings,