Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context class #110

Closed
BobDean opened this issue Sep 12, 2015 · 7 comments
Closed

Context class #110

BobDean opened this issue Sep 12, 2015 · 7 comments
Labels
question Further information is requested

Comments

@BobDean
Copy link

BobDean commented Sep 12, 2015

why was the Context pattern chosen over a traditional Singleton pattern?

@dirk-thomas
Copy link
Member

From my point of view when using dependency injection (which the context object is) the code can usually be used more flexible. Especially when it comes to testing it is much easier with DI then when the code uses singletons internally (which the ROS 1 code relies on heavily).

Just a few random references on the topic:

@wjwwood
Copy link
Member

wjwwood commented Sep 14, 2015

It's also worth noting that if you do not specify a Context, nodes use a singleton Context.

@BobDean
Copy link
Author

BobDean commented Sep 14, 2015

thanks, i'm just trying to understand the ros2 code. The Context looked "weird". The links were very helpful. Why is the context not just a service locator (which is apparently also bad(tm) according to stack overflow)?

@wjwwood
Copy link
Member

wjwwood commented Sep 14, 2015

Well, the rclcpp::Context object is sort of a service locator, since you can get a sub-context of any type as a singleton. So nodes ask the context for information they need to complete the intra process task, which is part of the definition from wikipedia:

https://en.wikipedia.org/wiki/Service_locator_pattern

This pattern uses a central registry known as the "service locator", which on request returns the information necessary to perform a certain task.

However, this is also like a singleton, since the is no "register" step for the information, it is just created if it doesn't already exist. For example, this is how the intra process manager singleton is gotten by the nodes:

https://github.com/ros2/rclcpp/blob/release-alpha1/rclcpp/include/rclcpp/node_impl.hpp#L149-L150
https://github.com/ros2/rclcpp/blob/release-alpha1/rclcpp/include/rclcpp/context.hpp#L43-L73

Basically nodes which share a Context also share any "singleton" "services" with each other. The more traditional "service locator" pattern would involve either a plugin system (where code is lookup and loaded dynamically) or something like the Windows registry. You could consider the DDS global configuration file a service locator I guess, a la OpenSplice's OSPL_URI env variable.

My opinion is that the "service locator" pattern leads to centralized and hierarchical designs which lead to globals and configuration complexity. The Context pattern, which is used by may systems e.g. OpenGL, allows for composition and decoupling of the configuration for parts of the system. But if you can come up with a rational to organize it differently I'm willing to consider it.

@BobDean
Copy link
Author

BobDean commented Sep 14, 2015

Dirk's links led to a large amount of reading over the weekend, and there
are a lot of conflicting opinions on single class instance mechanims
(singletons, contexts, locators etc.) For example, I am pretty sure there
was an article stating contexts were evil, which I did not quite agree
with. I am just trying to understand the design decisions that have been
made and the rationale.

is it my job to close issues, or do you all do that?

On Mon, Sep 14, 2015 at 4:16 PM, William Woodall notifications@github.com
wrote:

Well, the rclcpp::Context object is sort of a service locator, since you
can get a sub-context of any type as a singleton. For instance, this is how
the intra process manager singleton is gotten by the nodes:

https://github.com/ros2/rclcpp/blob/release-alpha1/rclcpp/include/rclcpp/node_impl.hpp#L149-L150

https://github.com/ros2/rclcpp/blob/release-alpha1/rclcpp/include/rclcpp/context.hpp#L43-L73

Basically nodes which share a Context share a any "singleton" "services"
with each other. The more traditional "service locator" pattern would
involve either a plugin system (where code is lookup and loaded
dynamically) or something like the Windows registry. You could consider the
DDS global configuration file a service locator I guess, a la OpenSplice's
OSPL_URI env variable.

My opinion is that the "service locator" pattern leads to centralized and
hierarchical designs which lead to globals and configuration complexity.
The Context pattern, which is used by may systems e.g. OpenGL, allows for
composition and decoupling of the configuration for parts of the system.
But if you can come up with a rational to organize it differently I'm
willing to consider it.


Reply to this email directly or view it on GitHub
#110 (comment).

@wjwwood
Copy link
Member

wjwwood commented Sep 14, 2015

@BobDean the feedback is welcome! Design patterns are rarely agreed on unanimously. The context seems appropriate here to me, and my intuition is to continue until we have cause to change direction.

Please close an issue if your question is resolved. We'll eventually come through and close them with a note to comment if that's in error.

@dirk-thomas dirk-thomas added the question Further information is requested label Sep 15, 2015
@wjwwood wjwwood closed this as completed Jun 23, 2016
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this issue Aug 5, 2022
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants