Skip to content

Implement Singleton Auxiliary class#20

Merged
juanlofer-eprosima merged 7 commits intomainfrom
feature/singleton
Nov 28, 2022
Merged

Implement Singleton Auxiliary class#20
juanlofer-eprosima merged 7 commits intomainfrom
feature/singleton

Conversation

@jparisu
Copy link
Copy Markdown
Contributor

@jparisu jparisu commented Nov 16, 2022

Signed-off-by: jparisu javierparis@eprosima.com

@jparisu jparisu temporarily deployed to codecov November 16, 2022 11:01 Inactive
@jparisu jparisu temporarily deployed to codecov November 16, 2022 11:01 Inactive
@jparisu jparisu temporarily deployed to codecov November 16, 2022 11:01 Inactive
@jparisu jparisu temporarily deployed to codecov November 16, 2022 11:01 Inactive
@jparisu jparisu marked this pull request as ready for review November 16, 2022 15:16
@jparisu jparisu temporarily deployed to codecov November 16, 2022 15:16 Inactive
@jparisu jparisu temporarily deployed to codecov November 16, 2022 15:16 Inactive
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Base: 73.04% // Head: 73.22% // Increases project coverage by +0.18% 🎉

Coverage data is based on head (a9f11a3) compared to base (96a49df).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
+ Coverage   73.04%   73.22%   +0.18%     
==========================================
  Files          42       43       +1     
  Lines         742      747       +5     
  Branches      175      175              
==========================================
+ Hits          542      547       +5     
  Misses        104      104              
  Partials       96       96              
Impacted Files Coverage Δ
...p_utils/include/cpp_utils/types/impl/Singleton.ipp 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jparisu jparisu temporarily deployed to codecov November 16, 2022 15:54 Inactive
@jparisu jparisu temporarily deployed to codecov November 16, 2022 15:54 Inactive
@jparisu jparisu temporarily deployed to codecov November 21, 2022 07:03 Inactive
@jparisu jparisu temporarily deployed to codecov November 21, 2022 07:03 Inactive
Comment thread cpp_utils/include/cpp_utils/types/Singleton.hpp Outdated
Comment thread cpp_utils/include/cpp_utils/types/Singleton.hpp Outdated
Comment thread cpp_utils/include/cpp_utils/types/Singleton.hpp Outdated
Comment thread cpp_utils/test/unittest/types/singletonTest.cpp
Comment thread cpp_utils/test/unittest/types/singletonTest.cpp Outdated
Comment thread cpp_utils/test/unittest/types/singletonTest.cpp
Comment thread cpp_utils/include/cpp_utils/types/Singleton.hpp
@jparisu jparisu temporarily deployed to codecov November 25, 2022 07:58 Inactive
@jparisu jparisu temporarily deployed to codecov November 25, 2022 07:58 Inactive
Comment thread cpp_utils/include/cpp_utils/types/impl/Singleton.ipp
template <typename T, int Index>
std::shared_ptr<T> Singleton<T, Index>::get_shared_instance() noexcept
{
static std::shared_ptr<T> instance_(new T());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to leave here our offline conversation:

Getting a pointer instead of a reference to an object in stack might be more dangerous (could be deleted by user), we need to have this in mind when using it and modify it if needed.

Copy link
Copy Markdown
Contributor Author

@jparisu jparisu Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, this way allows to control the liveliness of the internal object by shared_ptr, while having it in stack would not.
However, I still think that auto x = shared_ptr<int>(3); delete x.get(); is a possible code, and shared_ptr does not care. We might give the user some credit and trust its intelect 😋 .

jparisu added 6 commits November 28, 2022 11:24
Signed-off-by: jparisu <javierparis@eprosima.com>
Signed-off-by: jparisu <javierparis@eprosima.com>
Signed-off-by: jparisu <javierparis@eprosima.com>
Signed-off-by: jparisu <javierparis@eprosima.com>
Signed-off-by: jparisu <javierparis@eprosima.com>
Signed-off-by: jparisu <javierparis@eprosima.com>
@jparisu jparisu temporarily deployed to codecov November 28, 2022 10:24 Inactive
@jparisu jparisu temporarily deployed to codecov November 28, 2022 10:24 Inactive
Signed-off-by: jparisu <javierparis@eprosima.com>
@jparisu jparisu temporarily deployed to codecov November 28, 2022 10:38 Inactive
@jparisu jparisu temporarily deployed to codecov November 28, 2022 10:38 Inactive
Copy link
Copy Markdown
Contributor

@juanlofer-eprosima juanlofer-eprosima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@juanlofer-eprosima juanlofer-eprosima merged commit 48cf278 into main Nov 28, 2022
@juanlofer-eprosima juanlofer-eprosima deleted the feature/singleton branch November 28, 2022 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants