[GitHub] [incubator-doris] lingbin commented on a change in pull request #2969: Improve implementation of Status

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

lingbin commented on a change in pull request #2969: Improve implementation of Status
URL: https://github.com/apache/incubator-doris/pull/2969#discussion_r382657616
 
 

 ##########
 File path: be/src/common/status.h
 ##########
 @@ -248,49 +256,42 @@ class Status {
 };
 
 // some generally useful macros
-#define RETURN_IF_ERROR(stmt) \
-    do { \
+#define RETURN_IF_ERROR(stmt)            \
+    do {                                 \
         const Status& _status_ = (stmt); \
-        if (UNLIKELY(!_status_.ok())) { \
-            return _status_; \
-        } \
-    } while (false)
-
-#define RETURN_IF_STATUS_ERROR(status, stmt) \
-    do { \
-        status = (stmt); \
-        if (UNLIKELY(!status.ok())) { \
-            return; \
-        } \
+        if (UNLIKELY(!_status_.ok())) {  \
+            return _status_;             \
+        }                                \
     } while (false)
 
-#define EXIT_IF_ERROR(stmt) \
-    do { \
-        const Status& _status_ = (stmt); \
-        if (UNLIKELY(!_status_.ok())) { \
-            string msg = _status_.get_error_msg(); \
-            LOG(ERROR) << msg;            \
-            exit(1); \
-        } \
+#define EXIT_IF_ERROR(stmt)                         \
 
 Review comment:
   This macro is basically unchanged, so there is no maintenance issue. 
   
   Make `\` on the same column on the right, the code will be clearer, because readers **only need to focus on the implementation on the left** (same as the function). This macro is relatively small, and the advantages are not obvious.
   
   On the other hand, putting `\` in the same column, in addition to being easy to read, also has another advantage, especially the macros that often change, **that it is easy to find errors**, because someone may forget to write `\`. In extreme cases, sometimes compile still will succeed even missing `\`.
   
   Of course, as you said, there is no free lunch. The only problem with this alignment is that the number of diff lines may increase when the code is changed. But I think this "readability" and "easy to find errors" are more important.  :-)
   
   Anyway, it is a style problem, if you don't agree, I can change it back.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org