pnoltes commented on PR #721: URL: https://github.com/apache/celix/pull/721#issuecomment-1925867164
> > The first round review finished. I noticed code duplications in `celix_properties`, and thus suggest an alternative design: > > > > * Add only one type to `celix_properties`, i.e. ArrayList > > * Add only four (or more?) methods `setArrayList`/`assignArrayList`/`getArrayList`/`getAsArrayList` > > * Move element type information into `celix_array_list`, so that we can have the right copy operation for them. > > > > This way: > > > > * We don't need to check arrayList's element types, which is totally left to arrayList > > * The dangling pointer issue is solved naturally by the right copy operation in arrayList > > * The code duplication in `celix_properties` is eliminated. > > > > WDYT? > > Sounds good and I will look into this. I am bit uncertain how this impact celix_array_list and its current usage. > > Currently the array list implementation has no notion a an element type and just provided access to a union element entry. A logically way to specify the array list entry type is during creations, but this impact all usage of `celix_arrayList_create` and `celix_arrayList_createWithOptions`. > > An other options could be to set the element type when entries are added, so a call to `celix_arrayList_addLong` will make the array list element type long and a followup call to `celix_arrayList_setDouble` or `celix_arrayList_getDouble`, etc will fail. > > Or maybe mixed element types should be allowed, but I do no think this is a valid use case. I will create a separate pull request to add element type support to the array list, including a array list copy function. And when the array list with element type pull request is merged, I will update this PR and simplify the typed array list support for properties. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
