On 5/11/07, jerry gay <[EMAIL PROTECTED]> wrote:
On 5/10/07, Nicholas Clark <[EMAIL PROTECTED]> wrote: > On Thu, May 10, 2007 at 03:33:41AM -0500, Joshua Isom wrote: > > > > On May 9, 2007, at 4:01 PM, Nicholas Clark wrote: > > > >So, !s->strlen does scan as quickly and easily. > > > > > > > To some, but it isn't as easy to just literally read. "Not s's strlen" > > is a lot different than "STRING_IS_EMTPY". Since the code will be read > > often, and often by people not familiar with parrot's internals, it > > makes sense to make it easily readable. It takes me a second to read > > !s->strlen, but half a second to read STRING_IS_EMTPY. > > Whilst I agree with the .5s vs 1s, I'm still not convinced that I agree > [and we may have to agree to disagree] > > It comes down to the level of understanding of the internals. If every > construction is hidden behind macros that explain its function, then > I think it will help the beginners (as you say) and the knowledgeable > (who know what STRING_IS_EMPTY() expands to). > > But when I read STRING_IS_EMPTY() I stop and wonder "right, how?" and > stop to look up what it expands to. Which one does need to do, if one > is chasing down a bug. (Because with a bug, things *aren't* working as > at least one of the designer or implementor intended, which means > assumptions need to be checked. Maybe I'm odd) > since i wrote this code, i might as well come clean. i don't have a great understanding of the internals yet--at least, my c is still a bit rusty. while working on some code, i came across C<!(int)s->strlen> and a bunch of C<s == NULL>. i thought that since we have a PMC_IS_NULL() macro, we should have something similar for strings. using vim's tags support with the tags file generated by C<make tags> i looked up PMC_IS_NULL and found that it's not simply C<pmc == NULL>, which you might expect (however i still make few assumptions in c code, since my c is so rusty.) rather than figuring out why #if PARROT_CATCH_NULL PARROT_API extern PMC * PMCNULL; /* Holds single Null PMC */ # define PMC_IS_NULL(p) ((p) == PMCNULL || (p) == NULL) #else # define PMCNULL ((PMC *)NULL) # define PMC_IS_NULL(p) ((p) == PMCNULL) #endif /* PARROT_CATCH_NULL */ is the way it is, i decided to create STRING_IS_NULL and STRING_IS_EMPTY with the thought that maybe there's some room for magic in these new macros, like the one above. i figured somebody else could refine them, or that i'd take a look at it later when i had a better understanding of the above. however, i didn't comment the code, so shame on me. so, i was kinda leaving room for us to add a STRING *STRINGNULL or *STRINGEMPTY or something, if we saw fit. the extend/embed design is still incomplete, so i consider this an open item. > > >s == NULL is also more tersely written as !s, which, I feel, is also > > >clearer > > >to regular C programmers.
I would have to disagree on that one (although it's a matter of taste, I guess) What makes more sense "a string pointer that is not" or a string pointer that equals NULL? (in other words: !s or s == NULL/s != NULL) The same for comparisons with 0: a string length that is false (?) or a string length that equals 0? (in code: !s->length or s->length == 0) I think being more explicit (such as in "while (iter != NULL)" instead of "while (iter)") is better understandable. Instead of using macros (which can be a pain when debugging, as already noted earlier by somebody), it might be a good idea to add a comment saying what the code does :-) just my 2c. kjs
> > > Eh, if we have one, may as well have the other, although this one seems > > simple enough. > > STRING_IS_NULL() might not mean !s > !s can only mean !s > > > That's why I don't like it. > i agree, !s is much nicer on the eyes than s == NULL, and i would have converted it to that if i thought it was a good idea. however, once i came across the definition of PMC_IS_NULL, i coded the macro because i wasn't sure !s was sufficient. i figured that if there's room to correct the code, it can be done in one place, rather than replacing the many s == NULL and !s and who knows what else spread throughout the code. i'll happily accept ideas (or patches) on something better than what i coded. ~jerry
