[GitHub] [couchdb] davisp commented on a diff in pull request #4091: Optimize couch_util:to_hex/1

[GitBox <gi...@apache.org>]

davisp commented on code in PR #4091:
URL: https://github.com/apache/couchdb/pull/4091#discussion_r915060435


##########
src/couch/src/couch_util.erl:
##########
@@ -212,29 +212,36 @@ validate_utf8_fast(B, O) ->
             false
     end.
 
-to_hex(<<Hi:4, Lo:4, Rest/binary>>) ->
-    [nibble_to_hex(Hi), nibble_to_hex(Lo) | to_hex(Rest)];
-to_hex(<<>>) ->
-    [];
+to_hex(Binary) when is_binary(Binary) ->
+    binary_to_list(to_hex_bin(Binary));
 to_hex(List) when is_list(List) ->
     to_hex(list_to_binary(List)).

Review Comment:
   Do we need this clause to recurse back into to_hex/1 just to have the binary turned back into a list? It'd change the error a bit if List isn't convertible to binary I guess? This is just me thinking out loud, I don't have a direct preference really.



##########
src/couch/src/couch_util.erl:
##########
@@ -792,3 +799,31 @@ version_to_binary(Ver) when is_list(Ver) ->
     Ver1 = lists:reverse(lists:dropwhile(IsZero, lists:reverse(Ver))),
     Ver2 = [erlang:integer_to_list(N) || N <- Ver1],
     ?l2b(lists:join(".", Ver2)).
+
+-compile({inline, [hex/1]}).
+
+%% erlfmt-ignore
+hex(X) ->
+    % Table of all pairs of hex characters for a byte:

Review Comment:
   Might add a note that we're encoding the hex as ascii directly here. Took me a few minutes to realize what was going on.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org