-
Notifications
You must be signed in to change notification settings - Fork 290
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
This class feels so bloated. Can you split it up? #80
Comments
The functions are separated and sorted reasonably, and can be better used for other forms of verification, such as SMS verification codes, email verification codes, or other forms of verification. |
I don't understand this comment since the only goal of this class is to generate a captcha image, other methods should be implemented somewhere else. In other words, what would there be in common with SMS or email verification? About the bloat, maybe it is, I am open to PRs if you have better ideas |
I have n’t used this for a while, and I do n’t have a good understanding of the relationship chain. However, at least email and SMS verification does not require the transmission of results in the form of pictures, so the relevant function codes for the pictures in this class should be generated independently. As I have seen, this project is 7 years old. You understand this open source project better than me. I am a newcomer and I ca n’t understand the structure of it. Although there is not much code, I think if it is If it is split, it is safer for someone who is particularly familiar with the project structure. Glad you are willing to accept PR, open source, as you wish, to make him better together. |
After the function of generating pictures is independent, more other types of generating methods can be accepted.
|
Captcha/src/Gregwar/Captcha/CaptchaBuilder.php
Line 14 in 4bb668e
This class feels so bloated.
Can you split it up?
The text was updated successfully, but these errors were encountered: