Re: Rand Changes

Ok, We're now in a situation that has to be resolved. I think there are 3 issues: - The way php-dev behaved was flawed. While there's truth in what Sterling said about opensource projects, I have to say that the amount Jeroen put into the RFC about the subject is not very common. Thus, I can understand his anger, when php-dev suddenly wakes up after he's done all the work. - After looking at the code, I have to agree with Sterling about the style. Big chunked macros which should be functions, over-all breakage, etc., for something that is, after all, just supposed to return integers. It looks a bit like killing a fly with a cannon. - We can't ignore the fact that a much simpler and even less BC breaking way was offered - simply creating a new function named random(), unfortunately, after the work that's been done. In my opinion, if we combine these three into actions, then: - We owe an apology to Jeroen - We should revert the code and implement the KISS approach by simply introducing a new function. Zeev At 02:34 03-09-01, Sterling Hughes wrote:

    Howdy,

    I have a few problems with the latest random number related commits,
    most importantly being the quality of the underlying code, rather
    than some of the changes it implements (which are also un-necessary,
    imho, if it ain't broke -- don't "fix" it).

    1) On a style note, the code makes poor use of macro's,
    pim_srand_common is a good example of a macro gone wrong..

    2) It's un-optimized, and the code does not follow a logical
    structure, one indication of this is the PHP_FUNCTION_RAND() and
    PHP_RAND_RANGE(), which does too much for a macro in this case (and
    comments like that really don't belong in a macro).  Also, folks,
    don't use "register" on a variable that will be accessed twice.

    3) I don't think the extra level of abstraction is warranted in
    this case -- it slows things down, it doesn't speed them up, and
    there is really no great improvement from an API perspective to
    doing this.

    4) What was sooooooo wrong about the way we were doing things?

    Even though we don't have a voting system, I'm a strong -1 on this
    patch. :)))

    Instead -- changes that I think would be a good idea (and I'll
    implement these if you don't want to Jeroen) are:

    a) Making the seed argument optional to [mt_]srand(), and use the most
    random seed possible if no seed is specified, this is a good way to
    help out people.

    b) Update to the php4 api, and remove some of the redundant logic in rand.c
    pre-patch (well redundant from a PHP4 perspective).

    -Sterling

-- PHP Development Mailing List <http://www.php.net/> To unsubscribe, e-mail: php-dev-unsubscribe@lists.php.net For additional commands, e-mail: php-dev-help@lists.php.net To contact the list administrators, e-mail: php-list-admin@lists.php.net