SymfonyCasts/reset-password-bundle

Stop using private. Please use protected

FranzBruckner opened this issue · 2 comments

Hey guys,
thanks for this bundle.

To fulfill the story "As Admin i want to send an user a reset-password mail, independently of throttling" i had do tweak the PasswordHelper. Thanks to DI this is not a big deal by just overriding the service-definition for ResetPasswordHelper.

But extending SymfonyCasts\Bundle\ResetPassword\ResetPasswordHelper is a pain, because the use of private members/methods.

Yes, it will give users more power but will cause more problems to maintainers as they won't have that flexibility to easily change private methods without any BC breaks anymore. Also, just blindly using protected instead of private is a bad practice, every particular case should be considered thoroughly before revealing its accessibility. So, the question here is mostly about trade-offs and specific (edge) use cases

Howdy @FranzBruckner as @bocharsky-bw said, using protected instead of private for visibility is bad practice unless there is a very specific need to do so. You do have the ability to manipulate lifetime and the throttle_limit via the bundle configuration to suite the needs of your application. If there is a specific use case, that would benefit everyone, RFC's & PR's are always welcome.

Thanks for using the bundle!