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

STYLE: SingletonIndex does not need to store the unused func parameter #4164

Conversation

N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Aug 20, 2023

No longer stored the func parameter from SingletonIndex::SetGlobalInstance, and from Singleton(globalName, func, deleteFunc), as it was never actually retrieved from m_GlobalObjects afterwards.

Deprecated the SingletonIndex::SingletonData type alias, as it is no longer used internally anymore, and it should not be public either.


Note that even with the initial commit a66337e (pull request #118, Francois Budin (@fbudin69500) and Hans Johnson (@hjmjohnson), February 2019), the value of the func parameter was never retrieved from SingletonIndex::m_GlobalObjects, looking at:
https://github.com/InsightSoftwareConsortium/ITK/blob/a66337ec215c88f4900a2caf419b055483c42085/Modules/Core/Common/src/itkSingleton.cxx

https://github.com/InsightSoftwareConsortium/ITK/blob/a66337ec215c88f4900a2caf419b055483c42085/Modules/Core/Common/include/itkSingleton.h

@github-actions github-actions bot added area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) labels Aug 20, 2023
@N-Dekker N-Dekker force-pushed the SingletonIndex-remove-func branch 2 times, most recently from 9d3c6b3 to 8fc9b00 Compare August 20, 2023 13:45
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Aug 22, 2023

Still "draft", just because I would like to have it processed after another pull request #4162, because of possible merge conflicts between them.


Update, 5 September 2023: Ready for review now!

No longer stored the `func` parameter from `SingletonIndex::SetGlobalInstance`
and from `Singleton(globalName, func, deleteFunc)`, as it was never actually
retrieved from `m_GlobalObjects` afterwards.

Deprecated the `SingletonIndex::SingletonData` type alias, as it is no longer
used internally anymore, and it should not be public either.
@N-Dekker N-Dekker marked this pull request as ready for review September 5, 2023 20:19
@dzenanz dzenanz merged commit 6d1c4c7 into InsightSoftwareConsortium:master Sep 6, 2023
5 checks passed
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Sep 7, 2023
Added overloads of `SetGlobalInstance` and `Singleton` without the unused `func`
parameter. Let the new `SetGlobalInstance` overload just return `void`, instead
of `bool`. Deprecated the original overloads.

Follow-up to:

pull request InsightSoftwareConsortium#4164
commit 6d1c4c7
"STYLE: SingletonIndex does not need to store the unused `func` parameter"

pull request InsightSoftwareConsortium#4162
commit 6ec6328
"STYLE: Let `Singleton` assume that SetGlobalInstance always returns true"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Sep 14, 2023
Added overloads of `SetGlobalInstance` and `Singleton` without the unused `func`
parameter. Let the new `SetGlobalInstance` overload just return `void`, instead
of `bool`. Deprecated the original overloads, and made them
`ITK_FUTURE_LEGACY_REMOVE`.

Follow-up to:

pull request InsightSoftwareConsortium#4164
commit 6d1c4c7
"STYLE: SingletonIndex does not need to store the unused `func` parameter"

pull request InsightSoftwareConsortium#4162
commit 6ec6328
"STYLE: Let `Singleton` assume that SetGlobalInstance always returns true"
dzenanz pushed a commit that referenced this pull request Sep 18, 2023
Added overloads of `SetGlobalInstance` and `Singleton` without the unused `func`
parameter. Let the new `SetGlobalInstance` overload just return `void`, instead
of `bool`. Deprecated the original overloads, and made them
`ITK_FUTURE_LEGACY_REMOVE`.

Follow-up to:

pull request #4164
commit 6d1c4c7
"STYLE: SingletonIndex does not need to store the unused `func` parameter"

pull request #4162
commit 6ec6328
"STYLE: Let `Singleton` assume that SetGlobalInstance always returns true"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants