[issue15092] Using enum PyUnicode_Kind

[Terry J. Reedy]

Terry J. Reedy <tjreedy at udel.edu> added the comment:

Since we have defined an enum, I think we should use it consistently. (Am I correct in thinking it was added after the initial patches.) So I did a sanity check of -/+ lines for perhaps unintended changes. The only things I found to comment on, mostly somewhat picky, would *not* trigger test failures. 

===
-            int kind = PyUnicode_KIND(output);
+             kind = PyUnicode_KIND(output);
extra space added misaligns text (for humans)

===
+    default: assert(0);

I presume this is standard elsewhere for switches and just got omitted here.

-    }
-    assert(0);
-    return -1;
+    default:
+        assert(0);
+        return -1;
+    }
(two places)

Since assert(0) always fails, return can never happen (and was not added above. So I would think remove it. And I agree with putting it consistently under default: (which is a form of else) even if not required as it improves human readability.

===
-    int kind_self;
-    int kind_sub;
+    enum PyUnicode_Kind kind_self;
+    enum PyUnicode_Kind kind_sub;
     void *data_self;
     void *data_sub;

Several places elsewhere, multiple enum vars are defined on one line, so you *could* do the same here (and with void *vars?).

====
-    int kind1, kind2, kind;
+    enum PyUnicode_Kind kind1, kind2, kind;
<four places>
being really picky, I would put 'kind' first, not last, unless there is a good reason in the omitted context. It is mentally jarring for me as is without context. Elsewhere, things like src_/dest_/kind and x/y/z/kind are in the 'expected' order.

----------
nosy: +terry.reedy

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue15092>
_______________________________________