avro-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [avro] emkornfield commented on a change in pull request #1106: Core C++ implementation is modernized
Date Mon, 01 Mar 2021 04:19:37 GMT

emkornfield commented on a change in pull request #1106:
URL: https://github.com/apache/avro/pull/1106#discussion_r584437813



##########
File path: lang/c++/impl/ValidSchema.cc
##########
@@ -145,49 +140,49 @@ ValidSchema::toFlatList(std::ostream &os) const
  * in UTF-8 format. Note that this method is not responsible for validating
  * the schema.
  */
-string ValidSchema::compactSchema(const string& schema) {
-    bool insideQuote = false;
-    size_t newPos = 0;
-    string data(schema.data());
-
-    for (size_t currentPos = 0; currentPos < schema.size(); currentPos++) {
-        if (!insideQuote && std::isspace(data[currentPos])) {
-            // Skip the white spaces outside quotes.
-            continue;
-        }
+    string ValidSchema::compactSchema(const string &schema) {
+        auto insideQuote = false;

Review comment:
       That is fair.  I'm not sure I buy all of his arguments of Meyer's, I think it is highly
dependent assumed tooling..  I think when refactoring it doesn't really provide value to replace
the types, and can introduce bugs (as noted on the integer narrowing).  It seems like a good
practice for new code?




----------------------------------------------------------------
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.

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



Mime
View raw message