Community
Participate
Working Groups
In RSE, there are several different patterns of method names for getting singleton instances. I find these by doing a text search for "public static" in all java files and filtering to only show matches which include "()". For instance: SystemRegistry.getSystemRegistry() SystemProfileManager.getSystemProfileManager() DStoreConnectorServiceManager.getTheUniversalSystemManager() DaytimeConnectorServiceManager.getTheDaytimeConnectorServiceManager() SshConnectorServiceManager.getInstance() ArchiveHandlerManager.getInstance() FTPConnectorServiceManager.getDefault() I find these different patterns for accessing singleton instances very confusing. Before M6, we still have the chance to fix this and come up with a more common naming scheme. I personally like the getInstance() name best: SystemRegistry.getInstance() SystemProfileManager.getInstance() DStoreConnectorServiceManager.getInstance() DaytimeConnectorServiceManager.getInstance() Please comment on this request.
Sounds good to me. +1
I like getInstance() as well. The plugin classes themselves tend to use getDefault(), which I think we should keep for that particular case - otherwise getInstance() should be the one.
+ 1 I would like to see to go even a step further an would eliminate access methods in plugins and managers which only forward to an singleton instance getter method and access methods which forward to another plugin which forwards again to singleton instance getter method. Like: RSEUIPlugin.getThePersistenceManager() -> RSECorePlugin.getThePersistenceManager() -> getDefault().getPeristenceManager() - all of these calls could be just RSEPersistenceManager.getInstance() Or: SystemStartHere.getSystemProfileManager() -> SystemProfileManager.getSystemProfileManager() - There is no need for methods like SystemStartHere.getSystemProfileManager(). Singleton instance should be accessed via <SingletonClass>.getInstance() directly.
I agree, getInstance() is a far more standard, readable and predictable approach. +1
I'm not a committer, but I think it's a good idea. Everything that makes the use of RSE easier for the user is worth a change. +1
getInstance() is pretty common and also agree with Uwe's comments: +1
(In reply to comment #3) > I would like to see to go even a step further an would eliminate access > methods in plugins and managers which only forward to an singleton instance > getter method This cannot be done since many singletons belong to "internal" implementation, so they need public API access methods returning the corresponding interfaces. Examples for these are RSECorePlugin.getTheSystemRegistry() RSECorePlugin.getTheCoreRegistry() I'd like to keep the *.getThe*() naming scheme for these, in order to indicate that singletons are being returned. This may not be 100% Eclipse Standard, but it's a small deviation and I like it.
Created attachment 66655 [details] Refactoring Script for renamings Work completed, checkin comment: [177523][api] Unify singleton getter methods to use getInstance() In order to apply the changes to a local workspace, just choose Refactor > Apply Script... in Eclipse, and load attached refactoring script. Regular Expressions used for searching access methods: getThe getDefault getSingleton public\s+static\s+(\w*)\s+.*\1.*\(\) API Changes: ------------ DStoreConnectorServiceManager.getInstance() -- [was: getTheUniversalSystemManager()] RSECorePlugin.getTheCoreRegistry() -- [new] RSECorePlugin.getTheSystemRegistry() -- [new] RSECorePlugin.getThePersistenceManager() -- [moved from: RSEUIPlugin] RSEUIPlugin.getThePersistenceManager() -- [removed] SystemFileTransferModeRegistry.getInstance() -- [was: getDefault()] Non-API Changes: ---------------- DaytimeConnectorServiceManager.getInstance() -- [was: getTheDaytimeConnectorServiceManager()] DeveloperConnectorServiceManager.getInstance() -- [was: getTheDeveloperConnectorServiceManager()] FTPConnectorServiceManager.getInstance() -- [was: getDefault()] LocalConnectorServiceManager.getInstance() -- [was: getTheLocalSystemManager()] ServiceDiscoveryEngine.getInstance() -- [was: getServiceDiscoveryEngine()] SystemFilterStartHere.getInstance() -- [was: getDefault()] SystemRemoteEditManager.getInstance() -- [was: getDefault()] SystemUDASubstVarListCommon.getInstance() -- [was: getSingleton()] SystemViewDummyObject.getInstance() -- [was: getSingleton()] UDSubstListFolders.getInstance() -- [was: getSingleton()] UDSubstListCommonFiles.getInstance() -- [was: getSingleton()] UDSubstListFiles.getInstance() -- [was: getSingleton()] UniversalCompileSubstList.getInstance() -- [was: getSingleton()] Access methods not changed: --------------------------------- RSEUIPlugin.getTheSystemProfileManager() -- TBD separately when moving to Core RSEUIPlugin.getTheSystemRegistry() -- returns SystemRegistryUI SystemProfileManager.getDefault() -- remains since not really a singleton
Work completed. See the migration docs in comment #8 and the refactoring script, which you can apply to a local workspace, in attachment #66655 [details].
*** Bug 185097 has been marked as a duplicate of this bug. ***
[target cleanup] 2.0 M7 was the original target milestone for this bug
