Document and use the fact that `SingletonIndex::SetGlobalInstance` always returns true #4162

N-Dekker · 2023-08-19T17:44:24Z

Note that SetGlobalInstance has always just returned true, from the very first commit, pull request #118 commit a66337e ENH: Synchronize factories across modules in Python by Francois Budin (@fbudin69500) and Hans Johnson (@hjmjohnson), February 2019.

Looking at the initial (2019) version of SetGlobalInstance and SetGlobalInstancePrivate:

ITK/Modules/Core/Common/include/itkSingleton.h

Lines 62 to 64 in a66337e

    
           template<typename T> 
        
           bool SetGlobalInstance( const char * globalName, T * global, std::function<void(void*)> func, std::function<void(void)> deleteFunc) 
        
             { return this->SetGlobalInstancePrivate(globalName, global, func, deleteFunc);}

ITK/Modules/Core/Common/src/itkSingleton.cxx

Lines 97 to 104 in a66337e

    
           bool 
        
           SingletonIndex 
        
           ::SetGlobalInstancePrivate( const char * globalName, void * global, std::function<void(void*)> func, std::function<void(void)> deleteFunc) 
        
           { 
        
             m_GlobalObjects.erase(globalName); 
        
             m_GlobalObjects.insert(std::make_pair(globalName, std::make_tuple(global, func, deleteFunc) ) ); 
        
             return true; 
        
           }

Also fixed its comment.

`SingletonIndex::SetGlobalInstancePrivate` did always unconditionally return `true`, so it might as well just return `void`.

`SingletonIndex::SetGlobalInstance` always returns true, so it is not necessary to handle the hypothetical case that it /might/ return false.

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"

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"

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"

github-actions bot added the area:Core Issues affecting the Core module label Aug 19, 2023

N-Dekker added 4 commits August 19, 2023 20:39

STYLE: Use insert_or_assign in SingletonIndex SetGlobalInstancePrivate

5c14067

Also fixed its comment.

STYLE: Let SetGlobalInstancePrivate return void, instead of true

eadc465

`SingletonIndex::SetGlobalInstancePrivate` did always unconditionally return `true`, so it might as well just return `void`.

DOC: Document that SetGlobalInstance always returns true

9da299b

STYLE: Let Singleton assume that SetGlobalInstance always returns true

6ec6328

`SingletonIndex::SetGlobalInstance` always returns true, so it is not necessary to handle the hypothetical case that it /might/ return false.

N-Dekker force-pushed the SetGlobalInstance-returns-true branch from 01836ea to 6ec6328 Compare August 19, 2023 19:28

N-Dekker marked this pull request as ready for review August 20, 2023 08:52

dzenanz approved these changes Aug 21, 2023

View reviewed changes

N-Dekker mentioned this pull request Aug 22, 2023

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

Merged

dzenanz requested review from thewtex and blowekamp August 23, 2023 12:17

thewtex approved these changes Sep 5, 2023

View reviewed changes

thewtex merged commit 7fb08b8 into InsightSoftwareConsortium:master Sep 5, 2023
5 checks passed

N-Dekker mentioned this pull request Sep 7, 2023

STYLE: Remove unused parameter from SetGlobalInstance and Singleton #4209

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document and use the fact that `SingletonIndex::SetGlobalInstance` always returns true #4162

Document and use the fact that `SingletonIndex::SetGlobalInstance` always returns true #4162

N-Dekker commented Aug 19, 2023 •

edited

Loading

	template<typename T>
	bool SetGlobalInstance( const char * globalName, T * global, std::function<void(void*)> func, std::function<void(void)> deleteFunc)
	{ return this->SetGlobalInstancePrivate(globalName, global, func, deleteFunc);}

	bool
	SingletonIndex
	::SetGlobalInstancePrivate( const char * globalName, void * global, std::function<void(void*)> func, std::function<void(void)> deleteFunc)
	{
	m_GlobalObjects.erase(globalName);
	m_GlobalObjects.insert(std::make_pair(globalName, std::make_tuple(global, func, deleteFunc) ) );
	return true;
	}

Document and use the fact that SingletonIndex::SetGlobalInstance always returns true #4162

Document and use the fact that SingletonIndex::SetGlobalInstance always returns true #4162

Conversation

N-Dekker commented Aug 19, 2023 • edited Loading

Document and use the fact that `SingletonIndex::SetGlobalInstance` always returns true #4162

Document and use the fact that `SingletonIndex::SetGlobalInstance` always returns true #4162

N-Dekker commented Aug 19, 2023 •

edited

Loading