[ 
https://issues.apache.org/jira/browse/ORC-429?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16673552#comment-16673552
 ] 

ASF GitHub Bot commented on ORC-429:
------------------------------------

xndai closed pull request #333: ORC-429: [C++] Refactor code in TypeImpl.cc
URL: https://github.com/apache/orc/pull/333
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/c++/src/TypeImpl.cc b/c++/src/TypeImpl.cc
index 209ffcdfb9..d1d9c6203f 100644
--- a/c++/src/TypeImpl.cc
+++ b/c++/src/TypeImpl.cc
@@ -403,9 +403,6 @@ namespace orc {
     case proto::Type_Kind_STRUCT: {
       TypeImpl* result = new TypeImpl(STRUCT);
       std::unique_ptr<Type> return_value = std::unique_ptr<Type>(result);
-      uint64_t size = static_cast<uint64_t>(type.subtypes_size());
-      std::vector<Type*> typeList(size);
-      std::vector<std::string> fieldList(size);
       for(int i=0; i < type.subtypes_size(); ++i) {
         result->addStructField(type.fieldnames(i),
                                convertType(footer.types(static_cast<int>
@@ -644,32 +641,31 @@ namespace orc {
                                                        const std::string 
&input,
                                                        size_t start,
                                                        size_t end) {
-    std::string types = input.substr(start, end - start);
     std::vector<std::pair<std::string, ORC_UNIQUE_PTR<Type> > > res;
-    size_t pos = 0;
+    size_t pos = start;
 
-    while (pos < types.size()) {
+    while (pos < end) {
       size_t endPos = pos;
-      while (endPos < types.size() && (isalnum(types[endPos]) || types[endPos] 
== '_')) {
+      while (endPos < end && (isalnum(input[endPos]) || input[endPos] == '_')) 
{
         ++endPos;
       }
 
       std::string fieldName;
-      if (types[endPos] == ':') {
-        fieldName = types.substr(pos, endPos - pos);
+      if (input[endPos] == ':') {
+        fieldName = input.substr(pos, endPos - pos);
         pos = ++endPos;
-        while (endPos < types.size() && isalpha(types[endPos])) {
+        while (endPos < end && isalpha(input[endPos])) {
           ++endPos;
         }
       }
 
       size_t nextPos = endPos + 1;
-      if (types[endPos] == '<') {
+      if (input[endPos] == '<') {
         int count = 1;
-        while (nextPos < types.size()) {
-          if (types[nextPos] == '<') {
+        while (nextPos < end) {
+          if (input[nextPos] == '<') {
             ++count;
-          } else if (types[nextPos] == '>') {
+          } else if (input[nextPos] == '>') {
             --count;
           }
           if (count == 0) {
@@ -677,24 +673,24 @@ namespace orc {
           }
           ++nextPos;
         }
-        if (nextPos == types.size()) {
+        if (nextPos == end) {
           throw std::logic_error("Invalid type string. Cannot find closing >");
         }
-      } else if (types[endPos] == '(') {
-        while (nextPos < types.size() && types[nextPos] != ')') {
+      } else if (input[endPos] == '(') {
+        while (nextPos < end && input[nextPos] != ')') {
           ++nextPos;
         }
-        if (nextPos == types.size()) {
+        if (nextPos == end) {
           throw std::logic_error("Invalid type string. Cannot find closing )");
         }
-      } else if (types[endPos] != ',' && types[endPos] != '\0') {
+      } else if (input[endPos] != ',' && endPos != end) {
         throw std::logic_error("Unrecognized character.");
       }
 
-      std::string category = types.substr(pos, endPos - pos);
-      res.push_back(std::make_pair(fieldName, parseCategory(category, types, 
endPos + 1, nextPos)));
+      std::string category = input.substr(pos, endPos - pos);
+      res.push_back(std::make_pair(fieldName, parseCategory(category, input, 
endPos + 1, nextPos)));
 
-      if (nextPos < types.size() && (types[nextPos] == ')' || types[nextPos] 
== '>')) {
+      if (nextPos < end && (input[nextPos] == ')' || input[nextPos] == '>')) {
         pos = nextPos + 2;
       } else {
         pos = nextPos;


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Refactor code in TypeImpl.cc
> ----------------------------
>
>                 Key: ORC-429
>                 URL: https://issues.apache.org/jira/browse/ORC-429
>             Project: ORC
>          Issue Type: Improvement
>          Components: C++
>            Reporter: Fang Zheng
>            Priority: Minor
>
> Propose to make two changes to the code in TypeImpl.cc
>  
> 1. In convertType() function: in the case of proto::Type_Kind_STRUCT, two 
> vectors are created but never used. They shall be removed.
> 2. In TypeImpl::parseType() function: the function calls input.substr() to 
> copy the substring before parsing it. This string copy can be avoided by 
> directly parsing on the input string. Please see pull request for details.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to