You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/11/18 10:04:13 UTC
[GitHub] [incubator-doris] zuochunwei opened a new pull request #7145: no static_cast
zuochunwei opened a new pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145
provider interface in abstract-class replace static_cast
static_cast is not good for oop
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on a change in pull request #7145: no static_cast
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on a change in pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145#discussion_r752819977
##########
File path: be/src/exec/exec_node.h
##########
@@ -135,6 +135,12 @@ class ExecNode {
// each implementation should start out by calling the default implementation.
virtual Status close(RuntimeState* state);
+ virtual void set_num_senders(int num_senders) {}
Review comment:
when you need cast by hand writing in C++, it's almost means existed some design issues. try resolving this problem by cast is wrong, the problem of design should resolved by re-design.
when you hold a pointer or reference, but you want to use the interfaces designed for subclass, so the problem is type info missed, using cast is similar said to compiler: "don't trouble me, don't help me to check security" we should write code fit for compiler, not discard compiler, it's dangerous.
this viewpoint is not mine, but a major standpoint in OOP, you can find so much related articles, such as CppCoreGuildlines, edited by Bjarne Stroustrup（the father of C++） and Herb Sutter(C++ committee chairman)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #7145: no static_cast
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145#discussion_r752408214
##########
File path: be/src/exec/exec_node.h
##########
@@ -135,6 +135,12 @@ class ExecNode {
// each implementation should start out by calling the default implementation.
virtual Status close(RuntimeState* state);
+ virtual void set_num_senders(int num_senders) {}
Review comment:
I think it is not good to add the subclass method to the parent class.
These methods are indeed just for subclass.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei closed pull request #7145: no static_cast
Posted by GitBox <gi...@apache.org>.
zuochunwei closed pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] zuochunwei commented on a change in pull request #7145: no static_cast
Posted by GitBox <gi...@apache.org>.
zuochunwei commented on a change in pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145#discussion_r752819977
##########
File path: be/src/exec/exec_node.h
##########
@@ -135,6 +135,12 @@ class ExecNode {
// each implementation should start out by calling the default implementation.
virtual Status close(RuntimeState* state);
+ virtual void set_num_senders(int num_senders) {}
Review comment:
when you need cast by hand writing in C++, it's almost means existed some design issues. try resolving this problem by cast is wrong, the problem of design should resolved by re-design.
when you hold a pointer or reference, but you want to use the interfaces designed for subclass, so the problem is type info missed, using cast is similar said to compiler: "don't trouble me, don't help me to check security" we should write code fit for compiler, not discard compiler, it's dangours.
this viewpoint is not mine, but a major standpoint in OOP, you can find so much related articles, such as CppCoreGuildlines, edited by Bjarne Stroustrup（the father of C++） and Herb Sutter(C++ committee chairman)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #7145: no static_cast
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #7145:
URL: https://github.com/apache/incubator-doris/pull/7145#discussion_r755138282
##########
File path: be/src/exec/exec_node.h
##########
@@ -135,6 +135,12 @@ class ExecNode {
// each implementation should start out by calling the default implementation.
virtual Status close(RuntimeState* state);
+ virtual void set_num_senders(int num_senders) {}
Review comment:
OK, I see.
But I wonder is it possible to solve this problem by modifying `ExecNode::collect_scan_nodes` to return the real node type pointer by request?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org