search

RoboDK / RoboDK-API

Implementation of the RoboDK API in different programming languages. The RoboDK API allows simulating and programming any industrial robot (offline and online)
https://robodk.com/doc/en/RoboDK-API.html
Other
237 stars 117 forks source link

Implement Selection() and setSelection() #139

Closed vdm-dev closed 10 months ago

sjohnson-FLL commented 10 months ago

(originally from this forum post)

It looks like RoboDK_getItemList and Robodk_getItemListFilter are void functions, taking as argument a pointer to an int32_t where the number of filled items will be stored.

void RoboDK_getItemList(struct RoboDK_t *inst, struct Item_t *itemlist, int32_t itemlist_maxsize, int32_t *itemlist_sizeout);
void RoboDK_getItemListFilter(struct RoboDK_t *inst, const int32_t filter, struct Item_t *itemlist, int32_t itemlist_maxsize, int32_t *itemlist_sizeout);

RoboDK_Selection works similarly to the getItemList and getItemListFilter functions, but returns the number of filled items rather than taking a pointer to the number of filled items as an argument.

size_t RoboDK_Selection(struct RoboDK_t* inst, struct Item_t* items, size_t items_count);

In the interest of consistency, these three similar functions would ideally work the same: either all three would take a pointer or all three would return a value. It is more intuitive to me to return the number of filled items, like RoboDK_Selection does. However, I recognize that there are backwards-compatibility considerations that may make this challenging. It would be suboptimal to have to implement the following...

size_t RoboDK_getItemList2(struct RoboDK_t *inst, struct Item_t *itemlist, int32_t itemlist_maxsize);
size_t RoboDK_getItemListFilter2(struct RoboDK_t *inst, const int32_t filter, struct Item_t *itemlist, int32_t itemlist_maxsize);
vdm-dev commented 10 months ago

In the interest of consistency, these three similar functions would ideally work the same: either all three would take a pointer or all three would return a value. It is more intuitive to me to return the number of filled items, like RoboDK_Selection does. However, I recognize that there are backwards-compatibility considerations that may make this challenging. It would be suboptimal to have to implement the following...

RoboDK_Selection always returns the total count of selected items, not the number of filled items. I'm still not sure how it would be better.

My refusal to use the int32_t *itemlist_sizeout pointer is due to my desire to minimize the number of function arguments. But at the same time, I would not want to create copies of existing functions so as not to generate redundancy.

sjohnson-FLL commented 10 months ago

RoboDK_Selection always returns the total count of selected items, not the number of filled items. I'm still not sure how it would be better.

This was my understanding as well. By "number of filled items", I meant:

The number of items that were populated ("filled") into the array located at the struct Item_t *itemlist pointer.

This number would be equal to the total count of selected items.

Returning the number of selected items is, indeed, stylistically preferable to passing the pointer. The reason I suggested alternatives is to maintain consistency with the structure of existing similar functions like RoboDK_getItemList and RoboDK_getItemListFilter. Making the tradeoff of consistency vs. better structure is certainly a decision that you can and should be responsible for making. The reason I commented on this PR at all is to make sure you were aware that this structure does depart from the precedent -- sounds like you are aware of that.