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

[Paypal-payment] Bill Pugh Singleton Implementation in PaymentProviderHelper.class #731

Closed
wants to merge 4 commits into from

Conversation

LEHOANGGLAM
Copy link
Contributor

@LEHOANGGLAM LEHOANGGLAM commented Oct 14, 2023

  • Update the PaymentProviderHelper to Singleton Instance to avoid creating multiple PaymentProviderHelper instances and conserve memory

@LEHOANGGLAM LEHOANGGLAM changed the title [Paypal-payment] Update Lazy Initialization and Thread Safe Singleton in PaymentProviderHelper.class [Paypal-payment] Bill Pugh Singleton Implementation in PaymentProviderHelper.class Oct 14, 2023
@sonarcloud
Copy link

sonarcloud bot commented Oct 14, 2023

[payment-paypal] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@thiennn
Copy link
Contributor

thiennn commented Oct 15, 2023

I think the previous code is fine and simple. PAYPAL_PAYMENT_PROVIDER_ID is static and final. We don't need to create instance to access it

@thiennn thiennn closed this Oct 15, 2023
@khoahd7621
Copy link
Contributor

khoahd7621 commented Oct 15, 2023

In my perspective, apply singleton for PaymentProviderHelper is not necessary and cumbersome.
Static variable is class-based, not object-based. So static variable only loads one time when class is loaded at runtime.
Singleton will create a redundant object in memory for nothing purpose. The old implementation was fine.
Wait for the comments of anh @thiennn and anh @loinguyen-nt

@LEHOANGGLAM LEHOANGGLAM deleted the paypal-thread-safe-singleton branch February 26, 2024 08:35
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.

4 participants