> typedef struct {
>       char *hash;
>       int id;
>       int views;
> } Watch_t;

Why save the hash (key) string in the value record?  That's wasteful
unless you really need it.  In libJudy in general, the key/index is
saved in a very efficient form in the tree itself, and you get get all
of them using first/next functions, which tell you the indexes along the
way.  Unlike a lot of trees, you don't walk the tree to/through the
value nodes and then look up the index from the node itself...  It's not
saved there.

Or is hash your JudySL array itself?  Ugh, then what a misleading name
for it!

> Pvoid_t dict = (Pvoid_t) NULL;
>
> void init_track(Watch_t *track) {
>       assert(track);
>       track->hash = NULL;
>       track->id = 0; track->views = 0;
> }

OK, looks good so far.

> int set_to_storage(const unsigned char *key, Watch_t *val) {
>       PWord_t PV;
>
>       JSLI(PV, dict, key);
>       if (PV) {
>               PV = (PWord_t) val;
>               return 0;
>       } else {
>               return 1;
>       }
> }

Note that JSLI() won't return at all if there's a failure, unless you
revised JUDYERROR not to return (before #include'ing Judy.h).  So, you
need not test for nor handle the PV, just assume it's set after JSLI().
That's the main purpose for using the macros rather than functions.

And you must say *PV = (Word_t) val, not PV = (PWord_t) val.  You are
putting a value through a pointer.  The fact that val itself is a
pointer is not important here.  You have this backwards, don't store
your pointer in JudySL's pointer, store your pointer as a word UNDER the
JudySL pointer to value word.

Also, you really confused me here, is val, of type Watch_t, the object
you are malloc()'ing and storing under JudySL values?  Then I have no
idea what track->hash is.  Maybe you just left out comments that appear
in your original source code.

> Watch_t *get_from_storage(const unsigned char *key) {
>       PWord_t PV;
>
>       JSLG(PV, dict, key);
>
>       if (PV) {
>               return ((Watch_t *) PV);
>       } else {
>               return NULL;
>       }
> }

Here, checking PV is correct, but your return should be *PV, not PV.

By the way, this can be written MUCH more briefly and elegantly:

    return((Watch_t) (PV ? *PV : NULL));

> int main(int argc, char **argv) {
>       char *hash;
>       Watch_t *tr; Watch_t *track;
>       int bool;
>
>       hash = "t8xr5nk8jczy";
>
>       track = (Watch_t *) malloc( sizeof(Watch_t) );

Remember to check for malloc() failure.

>       init_track(track);
>
>       track->hash = hash;
>       track->id = 1; track->views = 16;

Why init the record if you immediately turn around and fill in all of
its values?

>       bool = set_to_storage((unsigned char *) hash, track);
>
>       tr = get_from_storage((unsigned char *) hash);
>       assert(tr);
>
>       fprintf(stderr, "Debug: hash=%s, track views=%d\n", tr->hash, 
> tr->views);
>
>     return 0;
> }

Alan

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables 
unlimited royalty-free distribution of the report engine 
for externally facing server and web deployment. 
http://p.sf.net/sfu/businessobjects
_______________________________________________
Judy-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/judy-devel

Reply via email to