-
-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added two new fuctions to dict API #63
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with this in concept, but see my notes.
Also, all APIs need to range check the input arguments - look for NULL pointers, etc.
pdfio-dict.c
Outdated
// | ||
|
||
size_t // O - Number of keys | ||
pdfioDictGetNumKeys(pdfio_dict_t *dict) // I - Dictionary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be pdfioDictGetNumPairs
.
pdfio-dict.c
Outdated
// | ||
// Alternative form to enumerate the keys of a dictionary | ||
|
||
bool // O - Value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should return the key string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem that I see only returning a string with the key It is that also It is of interest get the type with the save call. That left return as void or as bool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a function to get the type associated with a key. Alternately you could have a pdfio_valtype_t *
argument that (optionally) returns the type in the pointed variable. Returning the results via a struct pointer and separately returning a bool doesn't make a lot of sense.
pdfio-dict.c
Outdated
{ | ||
if (key){ | ||
key->key = dict->pairs[index].key; | ||
key->type = (pdfio_valtype_t **)(dict->pairs[index].value.type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dist->pairs[index].value.type
is a literal pdfio_valtype_t
value, not a pointer or pointer to pointer...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the way in that I manage to recall the value itself among the key string pointer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't make any sense. pdfio_valtype_t
is an enum (integer). You aren't returning the address of the enum, you are casting the enum value to a pointer which isn't valid.
pdfio.h
Outdated
@@ -123,6 +123,12 @@ typedef enum pdfio_valtype_e // PDF value types | |||
PDFIO_VALTYPE_NUMBER, // Number (integer or real) | |||
PDFIO_VALTYPE_STRING // String | |||
} pdfio_valtype_t; | |||
// Alternative PDF dict key enumeration | |||
typedef struct pdfio_dictKey_s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need this.
I'm not a full time programmer (only for hobby and some times for solve something in the work) and It is also my first "pull request" (also again in a very rudimentary way). The reason for this two functions It is no need the use of a callback for each time that review for a dictionary keys. |
pdfioDictGetNumKeys -> pdfioDictGetNumPairs pdfioDictGetKeyByIndex: - only return the key string and then only return a const char * test_mod.c: - changed to contemplate the requested modification
Add pdfioArrayRemove and pdfioDictClear APIs (Issue #74)
Hello,
I have added two new functions to iterate a dictionary without the use of a callback. I have also include a test purpose file.
Feel free to correct the changes. I think that are usefull.
P.D.: Sorry, I see in the commit that I have include the "test_mod" executable. Delete it.
Thanks.