Skip to content

[Enh]: Start if Needed on uniqueInstance Access #3

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

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

seandenigris
Copy link

No description provided.

@akgrant43
Copy link

Hi @seandenigris ,

I there's a few points to consider:

  • There's existing code that uses #uniqueInstance returning nil to test whether a new application instance should be created with specific configuration.
  • #start will raise an exception if called with the application started (with an inappropriate error message).
  • We can't be sure that this won't negatively affect other users.

Instead of modifying #uniqueInstance, I'd add another method, e.g. #runningInstance, that always returns a started instance.

What do you think?

@akgrant43 akgrant43 self-assigned this Apr 2, 2021
@seandenigris
Copy link
Author

  • There's existing code that uses #uniqueInstance returning nil to test whether a new application instance should be created with specific configuration.

Ah interesting. It sounds like the model is a bit more complex than I thought. That doesn't sound like the Singleton pattern as I understand it. I wonder if #uniqueInstance is even the right name. It may be misleading because it classically is sure to return something useful i.e. the singleton instance.

  • #start will raise an exception if called with the application started (with an inappropriate error message).

Wouldn't the application be sure not be started if #uniqueInstance is nil?

  • We can't be sure that this won't negatively affect other users.

Are there other users? Should I propose upstream and see if anyone objects?

Instead of modifying #uniqueInstance, I'd add another method, e.g. #runningInstance, that always returns a started instance.

I don't care too much, but lazy-initialized #uniqueInstance seems to be a common pattern and the "least surprising".

@akgrant43 akgrant43 merged commit f698e35 into feenkcom:master Apr 6, 2021
@akgrant43
Copy link

Hi Sean,

Wouldn't the application be sure not be started if #uniqueInstance is nil?

Doh! You're correct, of course.

I think that PBApplication started out as a singleton, however we want to be able to run multiple instances, but the class hasn't gone through the proper refactoring. So I've merged this in.

Thanks!

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.

2 participants