Extract the captcha & phrase builders to another repo

[Gregwar]

The ida of splitting logic was to extract the logic into another repo, in order to be able to use it for other purpose/framework than Sf2

Gregwar\Captcha ? Gregwar\CaptchaBuilder ?

[jeremylivingston]

Oh, I see. Sorry for the misunderstanding.

I don't think it's a big deal to have it in this repository. Someone could still use the ImageBuilder class directly without using the Symfony form type or generator class. For larger libraries, I can see the appeal of this, but I don't think it's a big deal at this point.

However, if we would like to do this, we could implement a structure similar to Assetic, Monolog, and Swiftmailer. These services have core libraries, but then separate repositories for the bundles.

I'm fine with either approach, to be honest.

[Gregwar]

No problem for misunderstanding, extracting the logic of image building is just the first step for extracting the class to another repo anyway ;-)

I'm sincerly convinced we should create another repo, because:

• With composer dependency, this would be really painless and the users won't make the difference
• This would allow people that are interested in the CAPTCHAs produced by the bundel to use it in other application, frameworks etc. (Silex extension? CakePHP plugin? ...)

Moreover, i think that this would be a perfect opportunity to focus on this bundle's main goal; creating images that only humans can read

If the bundle grows and more people use it in web application, we may someday have to deal with bots able to read our images, and this is why the image generation have to be more flexible, to allow a lot of parameters like:

• Customizing the colors
• Changing the scrambling (for instance, writing letters one by one with random angles) and the "waves power"
• Dealing with performance (disabling bilinear interpolation)
• Adding noise

The main system could be changed to make all CAPTCHA look more unique and hard to break. I think that could be done with random combinations of effects, we would first choose the way the text is written, then the font, then the scrambling, and the "overlay" (noise, lines etc.), and all of this among a wide range of possibilities. The effects combination has to be itself really wide and hard to guess,

[Gregwar]

So, I come back to this

I'm still hesitating to extract the image generation logic to another repo

[jeremylivingston]

I agree that it's probably a good idea, as long as it's flexible enough to remain framework-agnostic.

[Gregwar]

I created a new repository that contains the generator : https://github.com/Gregwar/Captcha/

But it's still not a dependency of this First, I clearly want to improve the quality and the difference of the images that are generated, and clean the code

I already changed a few things, you can have a look at the "demo" directory and open the index to view a lot of captchas, which are really differents right now :

[Gregwar]

When this will be cleaner and better, we'll make it a dependency for the bundle, and I may use the generator logic in another projects

[jeremylivingston]

This is awesome! Great work!

[jeremylivingston]

One thing that I noticed...There is a hard-coded dependency on PhraseBuilder in the CaptchaBuilder constructor. The PhraseBuilder service should be passed into the constructor, which will allow us to write unit tests for the service.

Also, we should avoid using a static CaptchaBuilder::create() method, as this also couples implementations to the CaptchaBuilder class. This will prevent people from extending it if they want to add functionality. We should remove this and require people to call a non-static create() method with the optional phrase.

Eventually, I'd like to write interfaces for all of these services, so we can allow for many different implementations. I can do this, if you'd like.

[jeremylivingston]

We will also want to add Symfony's Finder Component as a dependency for this package.

[Gregwar]

You're right for the PhraseBuilder. However, i'd really like to keep a default behaviour because this is really handy for the end-users to just instanciate, build and get the value without even think about the PhraseBuilder

I added the create() static method to allow one-line "instantiation + chaining" (see demo/ directory). But I agree that since PHP 5.4 you can do this with real instantiation. Maybe we should use late static binding to keep it working with extended class ? (just using static instead of self)

Providing interfaces is indeed a good idea, you can do it in this repo

I think we'll work on this and when this repo will seem ok we'll use it as a dependency of CaptchaBundle. I suggest we let this issue opened until this is merged, and we should create issues on the other repo for the pure captcha & phrase logics

[Gregwar]

I worked on the Captcha repository and it should be OK right now, ping @jeremylivingston , maybe we should make it a dependency to this ?

[jeremylivingston]

Looks good! Yeah, we can make it a composer dependency. I'm working on some other things this morning, but I can get to it after that and test it out.

Thanks for your hard work!

[jeremylivingston]

I just reviewed the changes that were made to the Captcha repository. Do we need to give the user the ability to provide the phrase as a constructor argument?

I think that we may want to encourage the user to use his/her own PhraseBuilder if they want to provide a custom set of phrases. Do you agree?

[Gregwar]

Ok, this is it, i've just added gregwar/captcha as a composer dependency ! Can you check that it's ok for you ?

[jeremylivingston]

Do you have any thoughts about my previous comment? I think that we should encourage developers to use their own implementation of the PhraseBuilderInterface if they want to provide different phrase requirements.

This way, we won't have to worry about a certain phrase getting bound to the class and causing inconsistencies. We'll always be able to refer back to the phrase builder to create a new phrase.

Do you agree with this change? If so, I can make the necessary adjustments.