On Tue, 4 Sep 2001, Joey Smith wrote:

> I realize I'm (as one person put it) "[stepping] right into the line of
> fire" on this, but I want to resolve the issues with Jeroen's big
> rand() patch.
>
> >From what I can tell, there are 3 camps:
>
>       1) Leave it, let Jeroen fix it.
>               We know it is broken, but we're willing to wait for
>               Jeroen to get through school orientation so that he
>               has time to fix it.
>

    Also known as the people who haven't taken a look at the source
    code.

>       2) Revert the whole big mess.
>               This camp seems to have an internal split. Some of them
>               hate the idea of the patch entirely for things like BC,
>               while others just seem upset with the way the patch was
>               merged with (what appears to be) very little "peer
>               review" (no matter who is at fault...I don't really
>               care. :) We'll call these "2A" and "2B" respectively.
>

    Actually, I think your missing me (do I get my own camp?)  and Zeev
    also agreed on this level I believe.  The design of the code is
    flawed, technically, I could care less about backwards
    compatibility.  My problem is that its implementation is technically
    poor and the design is making mountains out of molehills.

    Furthermore, its not solving a problem, just adding complexity.  How
    many people have complained to date about the "issues" this patch
    addresses.  I think Jeroen is the only one.

    Finally when a patch generates this much opposition, its really not
    something that should be in the code base.

>       3) Just like any other bug: Those who are bothered by it, fix it
>
>
> The reason I'm writing this is because I'm in the third camp. I could
> care less if it gets reverted or fixed, really...I just want PHP to
> *work*.
>

> If it's going to stay, we should *all* be able to pitch in and fix
> it...I really don't want to wait around. I wouldn't wait around when
> Sybase-CT was broken even *much* smaller ways...even when I had to teach
> myself C in my spare time! This is Open Source...if it's broken, I get
> to fix it! That's the whole *point*, isn't it?
>

    Well yes, but the flip side is also, if its broken, don't commit
    something.  What this patch is doing is taking something that worked
    perfectly well, and making it work less well/not work at all.  With no
    long term benefits on the horizon.  Its not like there is a good base
    from which to jump off from and improve.  It offers no benefits
    either.  If Jeroen didn't have CVS access do you honestly think
    anyone else who knows this code would've commited it?

> Personally, I'm not bothered by the way in which backwards compatibility
> is destroyed by this patch, because I think it actually behaves more
> like most people expect rand() to now...
>
> Good points:
> 1) no need to seed the generator
> 2) use mt_rand by default
>

    #1 can be done by at most a 15 line patch (probably less) to the
    old code, just add:

    if (!BG(rand_is_seeded)) {
        seed()
        BG(rand_is_seeded) = 1;
    }

    too the rand() functions, and

    BG(rand_is_seeded) = 1;

    to the srand() functions and your done.

    #2 This really is un-necessary, sure merner-twister is a better
    algorithm, but its not some *huge* improvement, for most people's
    needs the system rand() functions are perfectly acceptable.  For
    those who need merner-twister, they can simply use the mt_rand()
    functions.

    Discarding the fact that changing this really isn't a good idea
    (although its not imho horrible either way), this same functionality
    could be implemented in much fewer lines in a proper patch, maybe all
    in all both changes would be a 50 line patch, not an entire rand
    redesign.

    Believe, me, if it were as simple as improving upon his patch, I'd
    stop wasting time arguing this, and I'd go ahead and just fix it.
    The fact is though, pre-patch, the code was designed and implemented
    properly, after the patch, there is really no way I see to truly
    fix it, without reverting the underlying code.  The concept behind
    the changes is wrong from a technical perspective (not talking bc
    here), not just the implementation

> Bad points:
> 1) Leaks
> 2) Inconsistent style
> 3) Really bizzare macros, etc.
>

    And you don't see these as *serious* problems.  Is it really ok for
    people to commit code that has bad style, leaks and breaks the
    compile, and then complain about people who have issues with it?
    Btw, You also forgot:

    4) Slower
    5) Poor Design (fundamental concepts behind implementation)


> So, just to make my compile here happier (bcz I don't want to check out
> older CVS just to continue working), I have tried to fix some of the bad
> points.
>
> Since it is not clear to me yet which of the above camps is going to
> hold sway on this issue, I'm not going to commit them to CVS...but I
> *will* provides diffs for anyone who asks.
>
> After having thrown myself into the middle of a heated debate, I
> think I'm going to ignore php-dev for a few days until I'm ready to
> face the firestorm I know I'm stirring up. When the smoke clears,
> somebody please let me know. :)
>

    Please don't start an issue and then ignore it ;), I don't bite (well,
    I rarely bite).

    -Sterling



-- 
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]

Reply via email to