Re: [PEPr] Call for votes on Encryption::Crypt_RSA

Alexander Valyalkin wrote:

Alexander Valyalkin (http://pear.php.net/user/valyala) has initiated the call for votes on Encryption::Crypt_RSA. Alexander,

I think you went a bit too fast to switch to voting phase, I do have some comments to put before starting to vote: I propose that you put the following file and class structure in place, so to not clutter the Crypt_* name space and folder: Crypt_RSA (main class) |_ Crypt_RSA_Key |_ Crypt_RSA_KeyPair |_ Crypt_RSA_Math |_ Crypt_RSA_Math_BigInt |_ Crypt_RSA_Math_BCMath Crypt/

      RSA.php
      RSA/
          Key.php
          KeyPair.php
          Math/
               BigInt.php
               BCMath.php

There are some debug functions at the bottom of BCMath wrapper file. You still have a bunch of exit() statements. Return a PEAR_Error object instead (i.e. PEAR::raiseError(...)) You should use a &factory() method to specify which math wrapper to use. Something like: $rsa =& Crypt_RSA::factory($wrapperName, $params); then factory would do a PEAR::loadExtension($wrapperName), and then you can include_once 'Crypt/RSA/Math' . $wrapperName . '.php' and so on... That allows you to return a PEAR_Error object if no wrapper class or extension can be found. You could also let the system guess a wrapper automatically: $rsa =& Crypt_RSA::factory('', $params); with function &factory($wrapperName, $params) {

    if ($wrapperName == '') {
        // do some guessing logic here:
        // try to load bigint extension then try to load wrapper
        // Ok? no, then try BCMath extension instead...
        // If a wrapper cannot be found, then return a PEAR_Error
    }

} Also, to avoid loading files that may not be used, I generally do the require_once inside the constructor. I don't know if there is a PEAR policy on that though... Another solution could also to use calls to PEAR::getStaticProperty('Crypt_RSA') to store the wrapper for all the Crypt_RSA* classes. You could do something like: $result = Crypt_RSA::useWrapper($wrapperName); or $result = Crypt_RSA::useWrapper($wrapperInstance); with function useWrapper($wrapper) {

    $opt =& PEAR::getStaticProperty('Crypt_RSA', 'wrapper');
    if (is_string($wrapper)) {
        // Attempt to load wrapper...
    } else // test if $wrapper is an instance of Crypt_RSA_Math_*...
    }
    $opt = $wrapper;

} All this is just sample code to give you an idea of you can make that work, don't take it as is... A combination of factory() and useWrapper() is probably the most flexible and less troublesome to the end user. -Philippe