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.
>
> 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
